Skip to content

Commit

Permalink
test: More Page Models for Experiment Tracking [INFENG-694] (#9367)
Browse files Browse the repository at this point in the history
  • Loading branch information
JComins000 authored May 15, 2024
1 parent a96cafd commit 53edec9
Show file tree
Hide file tree
Showing 28 changed files with 470 additions and 83 deletions.
17 changes: 12 additions & 5 deletions webui/react/src/components/ColumnPickerMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,15 @@ const ColumnPickerTab: React.FC<ColumnTabProps> = ({
({ index, style }: { index: number; style: React.CSSProperties }) => {
const col = filteredColumns[index];
return (
<div className={css.rows} key={col.column} style={style}>
<div
className={css.rows}
data-test="row"
data-test-id={col.column}
key={col.column}
style={style}>
<Checkbox
checked={checkedColumn.has(col.column)}
data-test="checkbox"
id={col.column}
onChange={handleColumnChange}>
{col.displayName || col.column}
Expand All @@ -193,16 +199,17 @@ const ColumnPickerTab: React.FC<ColumnTabProps> = ({
);

return (
<div>
<div data-test-component="columnPickerTab">
<Input
allowClear
autoFocus
data-test="search"
placeholder="Search"
value={searchString}
onChange={handleSearch}
/>
{totalColumns.length !== 0 ? (
<div className={css.columns}>
<div className={css.columns} data-test="columns">
{filteredColumns.length > 0 ? (
<List height={360} itemCount={filteredColumns.length} itemSize={30} width="100%">
{rows}
Expand All @@ -216,10 +223,10 @@ const ColumnPickerTab: React.FC<ColumnTabProps> = ({
)}
{!settings.compare && (
<div className={css.actionRow}>
<Button type="text" onClick={handleShowHideAll}>
<Button data-test="showAll" type="text" onClick={handleShowHideAll}>
{allFilteredColumnsChecked ? 'Hide' : 'Show'} all
</Button>
<Button type="text" onClick={handleShowSuggested}>
<Button data-test="reset" type="text" onClick={handleShowSuggested}>
Reset
</Button>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,30 @@ interface Props {
const ConjunctionContainer = ({ index, conjunction, onClick }: Props): JSX.Element => {
return (
<>
{index === 0 && <div className={css.operator}>Where</div>}
{index === 0 && (
<div className={css.operator} data-test="where">
Where
</div>
)}
{index === 1 && (
<Select searchable={false} value={conjunction} width={'100%'} onChange={onClick}>
<Select
data-test="conjunction"
searchable={false}
value={conjunction}
width={'100%'}
onChange={onClick}>
{Object.values(Conjunction).map((c) => (
<Option key={c} value={c}>
{c}
</Option>
))}
</Select>
)}
{index > 1 && <div className={css.operator}>{conjunction}</div>}
{index > 1 && (
<div className={css.operator} data-test="conjunctionContinued">
{conjunction}
</div>
)}
</>
);
};
Expand Down
10 changes: 8 additions & 2 deletions webui/react/src/components/FilterForm/components/FilterField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,10 @@ const FilterField = ({
formStore.setFieldConjunction(parentId, (value?.toString() ?? 'and') as Conjunction);
}}
/>
<div className={css.fieldCard} ref={preview}>
<div className={css.fieldCard} data-test="fieldCard" ref={preview}>
<Select
autoFocus
data-test="columnName"
dropdownMatchSelectWidth={300}
options={columns.map((col, idx) => ({
key: `${col.column} ${idx}`,
Expand All @@ -201,6 +202,7 @@ const FilterField = ({
onChange={onChangeColumnName}
/>
<Select
data-test="operator"
options={(isSpecialColumn
? [Operator.Eq, Operator.NotEq] // just Eq and NotEq for Special column
: AvailableOperators[currentColumn?.type ?? V1ColumnType.UNSPECIFIED]
Expand All @@ -223,6 +225,7 @@ const FilterField = ({
{isSpecialColumn ? (
<div onKeyDownCapture={captureEnterKeyDown}>
<Select
data-test="special"
options={getSpecialOptions(field.columnName as SpecialColumnNames)}
value={fieldValue ?? undefined}
width={'100%'}
Expand All @@ -238,6 +241,7 @@ const FilterField = ({
{(currentColumn?.type === V1ColumnType.TEXT ||
currentColumn?.type === V1ColumnType.UNSPECIFIED) && (
<Input
data-test="text"
disabled={
field.operator === Operator.IsEmpty || field.operator === Operator.NotEmpty
}
Expand All @@ -252,6 +256,7 @@ const FilterField = ({
{currentColumn?.type === V1ColumnType.NUMBER && (
<InputNumber
className={css.fullWidth}
data-test="number"
value={fieldValue != null ? Number(fieldValue) : undefined}
onChange={(val) => {
const value = val != null ? Number(val) : null;
Expand Down Expand Up @@ -279,11 +284,12 @@ const FilterField = ({
</>
)}
<Button
data-test="remove"
icon={<Icon name="close" size="tiny" title="Close Field" />}
type="text"
onClick={() => formStore.removeChild(field.id)}
/>
<Button type="text">
<Button data-test="move" type="text">
<div ref={drag}>
<Icon name="holder" size="small" title="Move field" />
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ const FilterForm = ({ formStore, columns, onHidePopOver }: Props): JSX.Element =
};

return (
<div className={css.base}>
<div className={css.base} data-test-component="FilterForm">
{Loadable.match(loadableData, {
Failed: () => null, // TODO inform user if data fails to load
Loaded: (data) => (
<>
<div className={css.header}>
<div className={css.header} data-test="header">
<div>Show experiments…</div>
<Toggle
checked={data.showArchived}
Expand All @@ -64,19 +64,22 @@ const FilterForm = ({ formStore, columns, onHidePopOver }: Props): JSX.Element =
<div className={css.buttonContainer}>
<div className={css.addButtonContainer}>
<Button
data-test="addCondition"
disabled={isButtonDisabled}
type="text"
onClick={() => onAddItem(FormKind.Field)}>
+ Add condition
</Button>
<Button
data-test="addConditionGroup"
disabled={isButtonDisabled}
type="text"
onClick={() => onAddItem(FormKind.Group)}>
+ Add condition group
</Button>
</div>
<Button
data-test="clearFilters"
type="text"
onClick={() => {
formStore.removeChild(data.filterGroup.id);
Expand Down
29 changes: 22 additions & 7 deletions webui/react/src/components/FilterForm/components/FilterGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,10 @@ const FilterGroup = ({
}

return (
<div className={`${css.base} ${level === 0 ? css.baseRoot : ''}`} ref={(node) => drop(node)}>
<div
className={`${css.base} ${level === 0 ? css.baseRoot : ''}`}
data-test-component="FilterGroup"
ref={(node) => drop(node)}>
{level > 0 && (
<ConjunctionContainer
conjunction={conjunction}
Expand All @@ -130,11 +133,14 @@ const FilterGroup = ({
}}
/>
)}
<div className={`${css.groupCard} ${css[`level${level}`]}`} ref={preview}>
<div className={css.header}>
<div
className={`${css.groupCard} ${css[`level${level}`]}`}
data-test="groupCard"
ref={preview}>
<div className={css.header} data-test="header">
{level > 0 && (
<>
<div>
<div data-test="explanation">
{group.conjunction === Conjunction.And ? (
<div>All of the following are true...</div>
) : (
Expand All @@ -145,20 +151,29 @@ const FilterGroup = ({
disabled={group.children.length > ITEM_LIMIT}
menu={menuItems}
onClick={onItemClick}>
<Button icon={<Icon name="add" size="tiny" title="Add field" />} type="text" />
<Button
data-test="add"
icon={<Icon name="add" size="tiny" title="Add field" />}
type="text"
/>
</Dropdown>
<Button
data-test="remove"
icon={<Icon name="close" size="tiny" title="Close Group" />}
type="text"
onClick={() => formStore.removeChild(group.id)}
/>
<div ref={drag}>
<Button icon={<Icon name="holder" size="small" title="Move group" />} type="text" />
<Button
data-test="move"
icon={<Icon name="holder" size="small" title="Move group" />}
type="text"
/>
</div>
</>
)}
</div>
<div className={css.children}>
<div className={css.children} data-test="children">
{group.children.map((child, i) => {
if (child.kind === FormKind.Group) {
return (
Expand Down
2 changes: 1 addition & 1 deletion webui/react/src/components/OptionsMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const OptionsMenu: React.FC<OptionProps> = ({ rowHeight, onRowHeightChang
);
return (
<Dropdown menu={dropdownItems} selectedKeys={[`rowHeight-${rowHeight}`]}>
<Button icon={icon} tooltip="Options" />
<Button data-test-component="OptionsMenu" icon={icon} tooltip="Options" />
</Dropdown>
);
};
29 changes: 15 additions & 14 deletions webui/react/src/e2e/models/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Playwright has [a writeup](https://playwright.dev/docs/pom) on how page models a

### What playwright POM is missing

The playwright API approach still loosely defines each component. Relying on text can be volitile, so it would be better to use accessibility or `data-testid`. Playwright POM also serves as an API rather than a **model**.
The playwright API approach still loosely defines each component. Relying on text can be volitile, so it would be better to use accessibility or `data-test`. Playwright POM also serves as an API rather than a **model**.

Our page models are meant to implement the precise structure of a page. This allows us to be specific in which components we represent. For exmaple, a login page might have different sign in boxes for straightforward authetication and for SSO. Or maybe a dashboard page has several tables. When we test for "a username input" or "the first row of the table", these models will provide a uniform, specific mechanism to clearly state which component we're using.

Expand Down Expand Up @@ -65,15 +65,15 @@ export class DeterminedAuth extends NamedComponent({ defaultselector: `div[data-
readonly #form = new BaseComponent({ parent: this, selector: `form` });
readonly username = new BaseComponent({
parent: this.#form,
selector: `input[data-testid="username"]`,
selector: `input[data-test="username"]`,
});
readonly password = new BaseComponent({
parent: this.#form,
selector: `input[data-testid="password"]`,
selector: `input[data-test="password"]`,
});
readonly submit = new BaseComponent({
parent: this.#form,
selector: `button[data-testid="submit"]`,
selector: `button[data-test="submit"]`,
});
...
}
Expand All @@ -84,27 +84,28 @@ export class DeterminedAuth extends NamedComponent({ defaultselector: `div[data-
`NamedComponent`s will have a `static defaultSelector` to use if a selector isn't provided. Since this property is static, we can use it to create elements with more details. Here's an example using an imaginary `DeterminedTable` component:

```js
readonly userTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + "[data-testid='userTable']" });
readonly roleTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + "[data-testid='roleTable']" });
readonly userTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + "[data-test='userTable']" });
readonly roleTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + "[data-test='roleTable']" });
```

## Practices around test hooks

When creating page models, you'll most likely want to author test hooks into the `src` model.

| Test Hook | Usage |
| ------------------------------------ | -------------------------------------------------------------------- |
| `data-test-component='my-component'` | Belongs at the top level element wrapping the component |
| `data-testid='my-componentid'` | Attributed to any _instances_ of components or any intrinsic element |
| Test Hook | Usage |
| ------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------- |
| `data-test-component='my-component'` | Belongs at the top level element wrapping the component |
| `data-test='component'` | Attributed to any _instances_ of components or any intrinsic element |
| `data-test-id='dynamic'` | Test Hook for dynamic data. Consider having many `data-test='row'`s and needing to differentiate them in some way other than order |

Looking back to the exmaple with the imaginary `DeterminedTable`, we want to enable this pattern:

```js
// DeterminedTable.defaultSelector = '[data-test-component="DetTable"]'
readonly userTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + '[data-testid="userTable"]' });
readonly roleTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + '[data-testid="roleTable"]' });
readonly userTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + '[data-test="userTable"]' });
readonly roleTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + '[data-test="roleTable"]' });
```

The component `DeterminedTable` would have `data-test-component="DetTable"` as a top level attribute, and instances would each get their own `data-testid`. This way, the static attribute and the instance attribute don't conflict with each other.
The component `DeterminedTable` would have `data-test-component="DetTable"` as a top level attribute, and instances would each get their own `data-test`. This way, the static attribute and the instance attribute don't conflict with each other.

Not every component needs a data-testid, but, in general, more is better. It's better to select for _"a duck named Hoffman"_ rather than "a duck" or "Hoffman".
Not every component needs a data-test, but, in general, more is better. It's better to select for _"a duck named Hoffman"_ rather than "a duck" or "Hoffman".
13 changes: 13 additions & 0 deletions webui/react/src/e2e/models/ant/DatePicker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { NamedComponent } from 'e2e/models/BaseComponent';

/**
* Returns a representation of the DatePicker component from Hew.
* This constructor represents the contents in hew/src/kit/DatePicker.tsx.
* @param {object} obj
* @param {CanBeParent} obj.parent - The parent used to locate this DatePicker
* @param {string} obj.selector - Used instead of `defaultSelector`
*/
export class DatePicker extends NamedComponent {
readonly defaultSelector = '[class^="DatePicker_base"]';
// TODO implement this
}
19 changes: 11 additions & 8 deletions webui/react/src/e2e/models/ant/Tabs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,20 @@ import { BaseComponent, NamedComponent } from 'e2e/models/BaseComponent';
*/
export class Tabs extends NamedComponent {
readonly defaultSelector = 'div.ant-tabs';

/**
* Returns a templated selector for children components.
* @param {string} id - tab id
*/
static selectorTemplateTabs(id: string): string {
return `div.ant-tabs-tab-btn[id$="${id}"]`;
}
readonly tablist = new BaseComponent({ parent: this, selector: '.ant-tabs-nav' });
readonly tabContent = new BaseComponent({
parent: this,
selector: '.ant-tabs-content-holder',
});

/**
* Returns a representation of a tab item with the specified id.
* @param {string} id - the id of the menu item
*/
tab(id: string): BaseComponent {
return new BaseComponent({
parent: this.tablist,
selector: `div.ant-tabs-tab-btn[id$="${id}"]`,
});
}
}
Loading

0 comments on commit 53edec9

Please sign in to comment.