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

Refactor filesPage page-object for assertion management #2619

Merged
merged 1 commit into from
Dec 11, 2019

Conversation

kiranparajuli589
Copy link
Contributor

Description

All the assertions from filesPage page-objects are moved to respective contexts.

Related Issue

Motivation and Context

Better Code = Life GooD

How Has This Been Tested?

CI

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

.waitForElementVisible('@versionsList')
.api.elements('xpath', this.elements.versionsList.selector, function (result) {
assert.strictEqual(expectedNumber, result.value.length)
isVersionsPresent: async function () {
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
isVersionsPresent: async function () {
getVersionsCount: async function () {

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!
🚀

@jasson99
Copy link
Contributor

jasson99 commented Dec 9, 2019

LGTM 👍

.api.elements('xpath', this.elements.versionsList.selector, function (result) {
assert.strictEqual(expectedNumber, result.value.length)
getVersionsCount: async function () {
let count
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let count
let count = 0

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!
🚀

Comment on lines 326 to 327
this.elements.versionsList.locateStrategy,
this.elements.versionsList.selector,
Copy link
Contributor

@haribhandari07 haribhandari07 Dec 9, 2019

Choose a reason for hiding this comment

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

if possible use this.elements('@versionsList')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

absolutely possible!
🚀

@dpakach
Copy link
Contributor

dpakach commented Dec 9, 2019

alright just fix pending changes requests

Copy link
Contributor

@jasson99 jasson99 left a comment

Choose a reason for hiding this comment

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

👍

@kiranparajuli589 kiranparajuli589 requested review from skshetry, jasson99 and haribhandari07 and removed request for jasson99 December 10, 2019 04:09
Copy link
Contributor

@dpakach dpakach left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@haribhandari07 haribhandari07 left a comment

Choose a reason for hiding this comment

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

LGTM, please squash the commits

Copy link
Member

@individual-it individual-it left a comment

Choose a reason for hiding this comment

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

squash before merging

@haribhandari07 haribhandari07 merged commit 3e987ef into master Dec 11, 2019
@delete-merged-branch delete-merged-branch bot deleted the assertion-refactor-files-page branch December 11, 2019 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA:team Status:Needs-Review Needs review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants