Skip to content
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

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
53c045e
(test)O3-2428: E2E test for appointment workflow
Vijaykv5 Feb 14, 2024
5c36eaa
Merge branch 'main' into test/O3-2428
Vijaykv5 Feb 15, 2024
23a14a7
Merge branch 'openmrs:main' into test/O3-2428
Vijaykv5 Feb 16, 2024
bc721c0
test for add appointment
Vijaykv5 Feb 16, 2024
ac24aba
added test for edit and cancel appointment
Vijaykv5 Feb 17, 2024
e0e6be8
Merge branch 'main' into test/O3-2428
Vijaykv5 Feb 18, 2024
09f4967
Merge branch 'openmrs:main' into test/O3-2428
Vijaykv5 Feb 21, 2024
7b0cdc6
Updated tests
Vijaykv5 Feb 21, 2024
bd19ac6
Merge branch 'openmrs:main' into test/O3-2428
Vijaykv5 Feb 22, 2024
f92e560
Minor changes made
Vijaykv5 Feb 22, 2024
20ceaa3
Merge branch 'main' into test/O3-2428
Vijaykv5 Feb 24, 2024
d811207
Merge branch 'main' into test/O3-2428
Vijaykv5 Feb 25, 2024
98e3e38
Merge branch 'openmrs:main' into test/O3-2428
Vijaykv5 Feb 26, 2024
a30e561
New changes made
Vijaykv5 Feb 26, 2024
61b963a
Merge branch 'main' into test/O3-2428
Vijaykv5 Feb 26, 2024
84dafc0
Merge branch 'openmrs:main' into test/O3-2428
Vijaykv5 Feb 28, 2024
41e5296
fixed the date format
Vijaykv5 Feb 28, 2024
297ead6
Merge branch 'main' into test/O3-2428
Vijaykv5 Mar 3, 2024
90e76d6
Merge branch 'openmrs:main' into test/O3-2428
Vijaykv5 Mar 5, 2024
ae74d4c
Merge branch 'openmrs:main' into test/O3-2428
Vijaykv5 Mar 6, 2024
eaa061d
Fixed E2E test
Vijaykv5 Mar 6, 2024
087fc57
Fixed E2E testing
Vijaykv5 Mar 6, 2024
38299a3
Added patient visit
Vijaykv5 Mar 6, 2024
a5a251b
Renamed to appointments.spec.ts
Vijaykv5 Mar 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions e2e/pages/appointments-page.ts
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`);
}
}
1 change: 1 addition & 0 deletions e2e/pages/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export * from './medications-page';
export * from './program-page';
export * from './vitals-and-biometrics-page';
export * from './visits-page';
export * from './appointments-page';
114 changes: 114 additions & 0 deletions e2e/specs/add-appointments.spec.ts
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);
Copy link
Member

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?

Copy link
Contributor Author

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

Copy link
Member

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.

});

test('Add appointment for a patient, edit the added appointment and cancel it', async ({ page, api }) => {
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest we re-title the test to

Add, edit and cancel an appointment

Copy link
Member

Choose a reason for hiding this comment

The 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:

  1. Add an appointment
  2. Edit an existing appointment
  3. Cancel an existing appointment

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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:

  1. Add an appointment
  2. Edit an existing appointment
  3. Cancel an existing appointment

@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?

Copy link
Member

Choose a reason for hiding this comment

The 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:

@jayasanka-sack what do you think concerning Ian's idea?

Copy link
Member

@jayasanka-sack jayasanka-sack Mar 7, 2024

Choose a reason for hiding this comment

The 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.

Copy link
Member

@kdaud kdaud Mar 7, 2024

Choose a reason for hiding this comment

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

I also think breaking into three tests would make it much simpler.

@jayasanka-sack will this apply across all tests with the add, edit and delete workflows? cc: @denniskigen

Copy link
Member

@jayasanka-sack jayasanka-sack Mar 7, 2024

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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 beforeEach() rather than a beforeAll(), especially since we're fundamentally only working on data attached to the patient record rather than the patient record itself).

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.

Copy link
Member

Choose a reason for hiding this comment

The 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');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await page.getByLabel('Select location').selectOption('Inpatient Ward');
await page.getByLabel('Select location').selectOption('Inpatient Ward');

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();
});
});
Loading