Skip to content

Commit

Permalink
Test for Todo App and addition of grid helper
Browse files Browse the repository at this point in the history
  • Loading branch information
Ryanseanlee committed Sep 26, 2023
1 parent 1c0ab2c commit 1667998
Show file tree
Hide file tree
Showing 8 changed files with 266 additions and 30 deletions.
1 change: 1 addition & 0 deletions client-app/src/desktop/tabs/examples/ExamplesTab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const appTile = hoistCmp.factory<ExamplesTabModel>(({app, model}) => {
title: app.title,
icon: app.icon,
compactHeader: true,
testId: app.title,
items: div({
className: 'tb-examples__app-tile__contents',
items: app.text
Expand Down
11 changes: 7 additions & 4 deletions client-app/src/examples/todo/TaskDialog.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {uses, hoistCmp} from '@xh/hoist/core';
import {hoistCmp, uses} from '@xh/hoist/core';
import {Icon} from '@xh/hoist/icon';
import {filler, hbox, vbox} from '@xh/hoist/cmp/layout';
import {panel} from '@xh/hoist/desktop/cmp/panel';
Expand Down Expand Up @@ -30,12 +30,13 @@ export const taskDialog = hoistCmp.factory({

const formPanel = hoistCmp.factory(() =>
panel({
item: form(
vbox({
item: form({
testId: 'task-dialog',
item: vbox({
items: [description(), dueDate()],
className: 'todo-form'
})
),
}),
bbar: bbar()
})
);
Expand Down Expand Up @@ -68,10 +69,12 @@ const bbar = hoistCmp.factory<TaskDialogModel>(({model}) =>
toolbar(
filler(),
button({
testId: 'cancel-task-edit-button',
text: 'Cancel',
onClick: () => model.close()
}),
button({
testId: 'save-task-button',
text: 'OK',
icon: Icon.check(),
disabled: !model.formModel.isValid,
Expand Down
4 changes: 2 additions & 2 deletions client-app/src/examples/todo/TaskDialogModel.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {HoistModel, managed} from '@xh/hoist/core';
import {FormModel} from '@xh/hoist/cmp/form';
import {required, lengthIs} from '@xh/hoist/data';
import {lengthIs, required} from '@xh/hoist/data';
import {LocalDate} from '@xh/hoist/utils/datetime';
import {observable, action, makeObservable} from '@xh/hoist/mobx';
import {action, makeObservable, observable} from '@xh/hoist/mobx';
import {TodoPanelModel} from './TodoPanelModel';

export class TaskDialogModel extends HoistModel {
Expand Down
10 changes: 9 additions & 1 deletion client-app/src/examples/todo/TodoPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ export const todoPanel = hoistCmp.factory({
ref: model.panelRef,
mask: 'onLoad',
tbar: tbar(),
items: [grid({agOptions: {suppressMakeColumnVisibleAfterUnGroup: true}}), taskDialog()],
items: [
grid({
testId: 'todo-grid',
agOptions: {suppressMakeColumnVisibleAfterUnGroup: true}
}),
taskDialog()
],
bbar: bbar()
});
}
Expand Down Expand Up @@ -49,11 +55,13 @@ const bbar = hoistCmp.factory<TodoPanelModel>(({model}) =>
}),
'-',
switchInput({
testId: 'show-completed-switch',
bind: 'showCompleted',
label: 'Show Completed'
}),
filler(),
button({
testId: 'reset-button',
icon: Icon.reset(),
intent: 'danger',
tooltip: 'Reset to example defaults.',
Expand Down
15 changes: 10 additions & 5 deletions client-app/src/examples/todo/TodoPanelModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,26 @@ export class TodoPanelModel extends HoistModel {
icon: Icon.add(),
text: 'New',
intent: 'success',
actionFn: () => this.taskDialogModel.openAddForm()
actionFn: () => this.taskDialogModel.openAddForm(),
testId: 'add-button'
});

editAction = new RecordAction({
icon: Icon.edit(),
text: 'Edit',
intent: 'primary',
recordsRequired: 1,
actionFn: () => this.taskDialogModel.openEditForm(this.selectedTasks[0])
actionFn: () => this.taskDialogModel.openEditForm(this.selectedTasks[0]),
testId: 'edit-button'
});

deleteAction = new RecordAction({
icon: Icon.delete(),
text: 'Remove',
intent: 'danger',
recordsRequired: true,
actionFn: () => this.deleteTasksAsync(this.selectedTasks)
actionFn: () => this.deleteTasksAsync(this.selectedTasks),
testId: 'remove-button'
});

toggleCompleteAction = new RecordAction({
Expand Down Expand Up @@ -129,7 +132,8 @@ export class TodoPanelModel extends HoistModel {
message = count === 1 ? `'${description}?'` : `${count} tasks?`,
confirmed = await XH.confirm({
title: 'Confirm',
message: `Are you sure you want to permanently remove ${message}`
message: `Are you sure you want to permanently remove ${message}`,
confirmProps: {testId: 'confirm-delete-button'}
});

if (!confirmed) return;
Expand Down Expand Up @@ -230,7 +234,8 @@ export class TodoPanelModel extends HoistModel {
},
actionFn: ({record}) => {
this.toggleCompleteAsync([record.data]);
}
},
testId: 'toggle-complete-action'
}
]
},
Expand Down
120 changes: 120 additions & 0 deletions playwright/hoist/GridHelper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import {Page} from '@playwright/test';
import {HoistPage} from './HoistPage';
import {PlainObject} from '@xh/hoist/core';

export class GridHelper {
readonly hoistPage: HoistPage;

This comment has been minimized.

Copy link
@ghsolomon

ghsolomon Sep 26, 2023

Contributor

Wonder if these should be private?

readonly testId: string;

constructor(hoistPage: HoistPage, testId: string) {

This comment has been minimized.

Copy link
@ghsolomon

ghsolomon Sep 26, 2023

Contributor

There's a fancy way to declare and assign members all in the constructor signature if they don't require any special handling, though I'm not sure we do it anywhere else. Would look like this:

constructor(private readonly hoistPage: HoistPage, private readonly testId: string) {}

This comment has been minimized.

Copy link
@Ryanseanlee

Ryanseanlee Sep 26, 2023

Author Contributor

For me I think its nice to have the members at the top to look at easily without having to look at the constructor but im not sure what is best

this.hoistPage = hoistPage;
this.testId = testId;
}

get page(): Page {
return this.hoistPage.page;
}

async getRecordCount() {
return this.page.evaluate(testId => {
return window.XH.getActiveModelByTestId(testId).store.count;

This comment has been minimized.

Copy link
@ghsolomon

ghsolomon Sep 26, 2023

Contributor

Nit - could make this a one-liner by removing the return keyword here

}, this.testId);
}

async ensureCount(count: number) {
const gridCount = await this.getRecordCount();
if (gridCount !== count)
throw new Error(`Found ${gridCount} records when ${count} is expected`);

This comment has been minimized.

Copy link
@ghsolomon

ghsolomon Sep 26, 2023

Contributor

Should we use the Hoist throwIf util here?

}

async getRowData(recordIdQuery: RecordIdQuery) {

This comment has been minimized.

Copy link
@ghsolomon

ghsolomon Sep 26, 2023

Contributor

Would be good to add the return types. I think this would be Promise<PlainObject>, but is that right? I know you said something about only being able to return primitives

if ('id' in recordIdQuery) {
return this.page.evaluate(
([testId, id]) => window.XH.getActiveModelByTestId(testId).store.getById(id).data,
[this.testId, recordIdQuery.id]
);
} else {
return this.page.evaluate(
([testId, agId]) =>
window.XH.getActiveModelByTestId(testId).store.allRecords.find(
it => it.agId === agId
).id,

This comment has been minimized.

Copy link
@ghsolomon

ghsolomon Sep 26, 2023

Contributor

Looks like this is returning the id and not the data.

[this.testId, recordIdQuery.agId]
);
}
}

async getRowAgId(query: RecordQuery) {

This comment has been minimized.

Copy link
@ghsolomon

ghsolomon Sep 26, 2023

Contributor

Could add return type here I think? Promise<string>?

if ('agId' in query) return query.agId;
return 'id' in query
? this.page.evaluate(
([testId, id]) => window.XH.getActiveModelByTestId(testId).getById(id).agId,
[this.testId, query.id]
)
: this.page.evaluate(
([testId, recordData]) =>
window.XH.getActiveModelByTestId(testId).store.allRecords.find(({data}) =>
_.isMatch(data, recordData)
).agId,
[this.testId, query.recordData] as const
);
}

async getRowId(query: RecordQuery) {

This comment has been minimized.

Copy link
@ghsolomon

ghsolomon Sep 26, 2023

Contributor

Returns Promise<string | number> I think?

if ('id' in query) return query.id;
return 'agId' in query
? this.page.evaluate(
([testId, agId]) =>
window.XH.getActiveModelByTestId(testId).store.allRecords.find(
it => it.agId === agId
).id,
[this.testId, query.agId]
)
: this.page.evaluate(
([testId, recordData]) =>
window.XH.getActiveModelByTestId(testId).store.allRecords.find(({data}) =>
_.isMatch(data, recordData)
).id,
[this.testId, query.recordData] as const
);
}

// select row with GridModel
async selectRow(query: RecordQuery) {
const id = await this.getRowId(query);
this.page.evaluate(
([testId, id]) => {
const gridModel = window.XH.getActiveModelByTestId(testId),
record = gridModel.store.getById(id);
gridModel.selectAsync(record);
},
[this.testId, id]
);
}

// Functions to click / double click on grid row
async clickRow(query: RecordQuery) {
const agId = await this.getRowAgId(query);
await this.page.getByTestId(this.testId).locator(`div[row-id="${agId}"]`).click();
}

async dblClickRow(query: RecordQuery) {
const agId = await this.getRowAgId(query);
await this.page.getByTestId(this.testId).locator(`div[row-id="${agId}"]`).dblclick();
}
}

type RecordQuery = IdQuery | AgIdQuery | RecordDataQuery;

This comment has been minimized.

Copy link
@ghsolomon

ghsolomon Sep 26, 2023

Contributor

Could dry this up slightly by making it RecordIdQuery | RecordDataQuery. But maybe what you have is clearer

type RecordIdQuery = IdQuery | AgIdQuery;

interface IdQuery {
id: string | number;
}

interface AgIdQuery {
agId: string;
}

interface RecordDataQuery {
recordData: PlainObject;
}
37 changes: 19 additions & 18 deletions playwright/hoist/HoistPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {AppModel} from '../../client-app/src/desktop/AppModel';
import {XHApi} from '@xh/hoist/core/XH';
import {StoreRecordId} from '@xh/hoist/data/StoreRecord';
import {PlainObject} from '@xh/hoist/core';
import { GridHelper } from './GridHelper';

export interface FilterSelectQuery {
testId: string;
Expand Down Expand Up @@ -33,16 +34,20 @@ export class HoistPage {
await this.waitForAppToBeRunning();
}

getGridHelper(testId: string): GridHelper{

This comment has been minimized.

Copy link
@ghsolomon

ghsolomon Sep 26, 2023

Contributor

Wonder if this method should be called createGridHelper since it's returning a newly constructed instance? get sort of implies to me that it's just an accessor, but maybe this is an established pattern?

return new GridHelper(this, testId)
}

get(testId: string) {
return this.page.getByTestId(testId);
}

async click(testId: string) {
await this.get(testId).click();
getByText(text: string) {
return this.page.getByText(text)
}

async expectText(testId: string, text: string) {
await expect(this.get(testId)).toHaveText(text);
async click(testId: string) {
await this.get(testId).click();
}

async fill(testId: string, value: string) {
Expand Down Expand Up @@ -98,20 +103,6 @@ export class HoistPage {
await this.page.locator('label', {has: this.page.getByTestId(testId)}).uncheck();
}

async getGridRowByRecordId(testId: string, id: StoreRecordId) {
return this.page.evaluate(() =>
window.XH.getActiveModelByTestId(testId).gridModel.store.getById(id)
);
}

async getGridRowByCellContents(testId: string, spec: PlainObject) {
return this.page.evaluate(() => {
window.XH.getActiveModelByTestId(testId).gridModel.store.allRecords.find(({data}) =>
_.isMatch(data, spec)
);
});
}

onConsoleError(msg: ConsoleMessage) {
throw new Error(msg.text());
}
Expand All @@ -133,6 +124,16 @@ export class HoistPage {
await expect(this.maskLocator).toHaveCount(0, {timeout: 10000});
}

//Expect
async expectText(testId: string, text: string) {
await expect(this.get(testId)).toHaveText(text);
}

async expectTextVisible(text: string) {
await expect(this.getByText(text)).toBeVisible({timeout: 10000})
}


//------------------------
// Implementation
//------------------------
Expand Down
Loading

1 comment on commit 1667998

@ghsolomon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good overall I think!

Please sign in to comment.