Skip to content
This repository has been archived by the owner on Feb 18, 2021. It is now read-only.

Try to make #changeToNewRole a bit more reliable #1013

Closed
wants to merge 1 commit into from

Conversation

brbrr
Copy link
Contributor

@brbrr brbrr commented Mar 8, 2018

There are 2 buttons with the same locators in the EditTeamMemberPage. 'Update' button is in disabled state it role was not changed.

This PR makes #changeToNewRole click only on active 'Update' button, also it chains the driver-related calls to guarantee synchronous execution

@brbrr brbrr self-assigned this Mar 8, 2018
Copy link
Contributor

@alisterscott alisterscott left a comment

Choose a reason for hiding this comment

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

Is this actually any different than what was there before?

I don't really understand why you'd need to chain these actions here?

@brbrr
Copy link
Contributor Author

brbrr commented Mar 8, 2018

It is different :)

So while I tried to debug CircleCI failures I noticed that Invites test fails after that method call (https://github.com/Automattic/wp-e2e-tests/blob/master/specs/wp-invite-users-spec.js#L706)
Not sure why, but the local run was clicking on the disabled button(before it selects 'Author' from dropdown)

So here I changed button locator since there 2 buttons located with button.form-button.is-primary and it should click only not disabled button.

I don't really understand why you'd need to chain these actions here?

In my understanding, all(or most) of the driver and driverHelper methods return a promise. And in my experience, WebDriverJS promise manager isn't reliable enough, that's why I prefer to chain all these promise calls whenever it's needed or not. As a side benefit - it would be much easier to convert then chains into await calls when we'll move to async/await style (#329)

@brbrr brbrr changed the title make #changeToNewRole a bit reliable Try to make #changeToNewRole a bit more reliable Mar 8, 2018
@brbrr brbrr closed this Mar 8, 2018
@brbrr
Copy link
Contributor Author

brbrr commented Mar 8, 2018

Its not needed

@brbrr brbrr deleted the try/make-changeToNewRole-reliable branch March 8, 2018 08:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants