-
Notifications
You must be signed in to change notification settings - Fork 248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(test) O3-2428: E2E test for appointment scheduling #1664
Changes from 18 commits
53c045e
5c36eaa
23a14a7
bc721c0
ac24aba
e0e6be8
09f4967
7b0cdc6
bd19ac6
f92e560
20ceaa3
d811207
98e3e38
a30e561
61b963a
84dafc0
41e5296
297ead6
90e76d6
ae74d4c
eaa061d
087fc57
38299a3
a5a251b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import { type Page } from '@playwright/test'; | ||
|
||
export class AppointmentsPage { | ||
constructor(readonly page: Page) {} | ||
|
||
readonly appointmentsTable = () => this.page.getByTestId('table'); | ||
|
||
async goto(uuid: string) { | ||
await this.page.goto(`/openmrs/spa/patient/${uuid}/chart/Appointments`); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,114 @@ | ||||||
import { expect } from '@playwright/test'; | ||||||
import { generateRandomPatient, deletePatient, type Patient } from '../commands'; | ||||||
import { test } from '../core'; | ||||||
import { AppointmentsPage } from '../pages'; | ||||||
|
||||||
let patient: Patient; | ||||||
|
||||||
test.beforeEach(async ({ api }) => { | ||||||
patient = await generateRandomPatient(api); | ||||||
}); | ||||||
|
||||||
test('Add appointment for a patient, edit the added appointment and cancel it', async ({ page, api }) => { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest we re-title the test to
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest this be broken up into three tests, as it's three features:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@ibacher we are following this doc for the tests. So for adding tests for appointment section, is it better to do it as a whole test for appointment section? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@jayasanka-sack what do you think concerning Ian's idea? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggested using a single flow to minimize API usage for the data population. But, after seeing how lengthy the test will be, I also think breaking it into three tests would simplify it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@jayasanka-sack will this apply across all tests with the add, edit and delete workflows? cc: @denniskigen There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We've got just a few that contains in single flow. Let's leave them as is for the time being and beef them up later when we can. Our priority is to expand our coverage. But, if you happen to have some extra time, feel free to tweak them too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's definitely a balance of concerns, but if the main concern is how much data is created during, it should be possible, for example, to create a single patient and run a suite of tests using that single patient (i.e., there's no reason here that the patient needs to be created in a My concern here is related to this point in that doc: "We want business-level users to easily be able to see a report that reassures them about what functionality is covered". Specifically, if the "Add appointment for a patient, edit the added appointment and cancel it" test fails, then, as a business user, I'm stuck regarding that whole feature as broken, which is much more concerning than saying "We can currently add appointments, but something broke the cancellation flow", for example. I also agree with @jayasanka-sack's point that the priority should be improving test coverage, not going back and revising everything to match some "gold standard" for how tests are written. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created a ticket to monitor the proposed E2E test approach. |
||||||
const appointmentsPage = new AppointmentsPage(page); | ||||||
|
||||||
await test.step('When I go to the appointment tab in the patient chart', async () => { | ||||||
await appointmentsPage.goto(patient.uuid); | ||||||
}); | ||||||
|
||||||
await test.step('And I click on the “Add” button', async () => { | ||||||
await page.getByRole('button', { name: 'Add', exact: true }).click(); | ||||||
}); | ||||||
|
||||||
await test.step('And I select Mobile Clinic location', async () => { | ||||||
await page.getByLabel('Select location').selectOption('Mobile Clinic'); | ||||||
}); | ||||||
|
||||||
await test.step('And I select “Outpatient Department” service', async () => { | ||||||
await page.getByLabel('Select a service').selectOption('Outpatient Department'); | ||||||
}); | ||||||
|
||||||
await test.step('And I make appointment as “Scheduled”', async () => { | ||||||
await page.getByLabel('Select an appointment type').selectOption('Scheduled'); | ||||||
}); | ||||||
|
||||||
await test.step('And I set date for tomorrow', async () => { | ||||||
const tomorrow = new Date(); | ||||||
tomorrow.setDate(tomorrow.getDate() + 1); | ||||||
await page.fill('input[placeholder="dd/mm/yyyy"]', tomorrow.toLocaleDateString('en-GB')); | ||||||
}); | ||||||
|
||||||
await test.step('And I set the “Duration” to 60', async () => { | ||||||
await page.getByLabel('Duration (minutes)').fill('60'); | ||||||
}); | ||||||
|
||||||
await test.step('And I add a note', async () => { | ||||||
await page.getByPlaceholder(/Write any additional points here/i).fill('Testing Appointments notes'); | ||||||
}); | ||||||
|
||||||
await test.step('And I click Save button', async () => { | ||||||
await page.getByRole('button', { name: /save and close/i }).click(); | ||||||
}); | ||||||
|
||||||
await test.step('Then I should see a success message', async () => { | ||||||
await expect(page.getByText(/Appointment scheduled/i)).toBeVisible(); | ||||||
}); | ||||||
|
||||||
await test.step('When I click the options kebab menu in the appointment', async () => { | ||||||
await page.getByRole('button', { name: 'Options' }).click(); | ||||||
}); | ||||||
|
||||||
await test.step('And I choose the "Edit" option from the popup menu', async () => { | ||||||
await page.getByRole('menuitem', { name: 'Edit' }).click(); | ||||||
}); | ||||||
|
||||||
await test.step('Then I should see the Edit an appointment workspace', async () => { | ||||||
await expect(page.getByText(/Edit an appointment/i)).toBeVisible(); | ||||||
}); | ||||||
|
||||||
await test.step('When I change to “Inpatient ward” location', async () => { | ||||||
await page.getByLabel('Select location').selectOption('Inpatient Ward'); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Can you check in other E2E tests how they select options in the tests. Test is failing because of it. Download the test report and check the video where it's failing. |
||||||
}); | ||||||
|
||||||
await test.step('And I change to “General Medicine” Service', async () => { | ||||||
await page.getByLabel('Select a service').selectOption('General Medicine service'); | ||||||
}); | ||||||
|
||||||
await test.step('And I change the date to Today', async () => { | ||||||
const today = new Date(); | ||||||
today.setDate(today.getDate()); | ||||||
await page.fill('input[placeholder="dd/mm/yyyy"]', today.toLocaleDateString('en-GB')); | ||||||
}); | ||||||
|
||||||
await test.step('And I set the “Duration” of the appointment”', async () => { | ||||||
await page.getByLabel('Duration (minutes)').fill('80'); | ||||||
}); | ||||||
|
||||||
await test.step('And I change the note', async () => { | ||||||
await page.getByPlaceholder(/Write any additional points here/i).fill('Editing Appointmentments notes'); | ||||||
}); | ||||||
|
||||||
await test.step('And I click Save button', async () => { | ||||||
await page.getByRole('button', { name: /save and close/i }).click(); | ||||||
}); | ||||||
|
||||||
await test.step('Then I should see a success message', async () => { | ||||||
await expect(page.getByText(/Appointment edited/i)).toBeVisible(); | ||||||
}); | ||||||
|
||||||
await test.step('When I click the options kebab menu in the appointment', async () => { | ||||||
await page.getByRole('button', { name: 'Options' }).click(); | ||||||
}); | ||||||
|
||||||
await test.step('And I choose the "Cancel" option ', async () => { | ||||||
await page.getByRole('menuitem', { name: 'Cancel' }).click(); | ||||||
}); | ||||||
|
||||||
await test.step('When I click the "Cancel appointment" button to confirm', async () => { | ||||||
await page.getByRole('button', { name: 'danger Cancel appointment' }).click(); | ||||||
}); | ||||||
|
||||||
await test.step('Then I should see a success message', async () => { | ||||||
await expect(page.getByText(/Appointment cancelled successfully/i)).toBeVisible(); | ||||||
}); | ||||||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we start patient visit before recording the appointment, @Vijaykv5 what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops I missed that out. Yes that right!
Thank you @kdaud
I've updated new changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appointments should be able to be scheduled without requiring an active visit. That's a kind of basic feature of an appointment scheduling application.