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

80-Test for Search functionality #7225

Merged
merged 4 commits into from
Aug 5, 2021
Merged

Conversation

meghna-khemka
Copy link
Contributor

@meghna-khemka meghna-khemka commented Aug 1, 2021

Description

Test the Search Functionality

medic/cht-core#[80]

Code review checklist

  • Readable: Concise, well named, follows the style guide, documented if necessary.
  • Documented: Configuration and user documentation on cht-docs
  • Tested: Unit and/or e2e where appropriate
  • Internationalised: All user facing text
  • Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@meghna-khemka meghna-khemka marked this pull request as ready for review August 2, 2021 03:55
Copy link
Contributor

@ngaruko ngaruko left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few comments. Especially with browser.pause()

@@ -49,6 +49,10 @@ describe('Create Person Under Area', async () => {
await loginPage.cookieLogin();
});

afterEach(async () => {
await utils.revertDb([], true);
Copy link
Contributor

Choose a reason for hiding this comment

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

We are planning of using these in wdi hooks. We seem to use it in every spec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure I guess we can keep it for now and when we add it in the hooks, we can come back and remove it. Does it make sense?

const commonPage = require('../../page-objects/common/common.wdio.page');
const placeFactory = require('../../factories/cht/contacts/place');
const personFactory = require('../../factories/cht/contacts/person');
const places = placeFactory.generateHierarchy(); // This generates ['district_hospital', 'health_center', 'clinic']
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you did not need to comment on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

const moment = require('moment');
const placeFactory = require('../../factories/cht/contacts/place');
const personFactory = require('../../factories/cht/contacts/person');
const places = placeFactory.generateHierarchy(); // This generates ['district_hospital', 'health_center', 'clinic']
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, about the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

await (await searchIcon()).click();
// After the button is pressed there can be a slight delay before the AJAX call
// is made and the search spinner shows up hence we just need to wait for a bit before moving forward
await browser.pause(1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

We normally try to avoid random wait/pause but rather be specific on what we want to wait for. Maybe someElement.waitForDisplayed({}), which can also be used for wait for element to disappear with `r{everse:true}'?

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 feel using a fixed wait time here actually makes sense because this wait is not dependent on any network call, it's just to make sure that after we click, we give enough time for the search AJAX call to trigger. Maybe waiting on spinner will make it more complex. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think if it is the spinner we are waiting to disappear we could use that. $('.loader')); which we probably need to do in many other areas, thus might need a sort of utility function for that. But not a biggie, so feel free to merge.

await (await clearContacts()).click();
// After the button is pressed there can be a slight delay before the AJAX call
// is made and the search spinner shows up hence we just need to wait for a bit before moving forward
await browser.pause(1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, the random pause

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants