E2E: Refactor clickWhenClickable() helper #50911

WunderBart · 2021-03-09T17:37:41Z

Changes proposed in this pull request

Refactored DriverHelper.clickWhenClickable() to make sure that element is enabled before clicking. This should address some general flakiness where a clicked element is (initially) disabled (due to i.e. pending request).
The above revealed an issue with the ensureMobileMenuOpen helper, where the findElement & clickWhenClickable methods weren't called via await and, as a result, returning an unreliable state of the menu. This PR fixes that.

Testing instructions

Specs should pass :)

matticbot · 2021-03-09T17:37:43Z

Test live: https://calypso.live/?branch=refactor/e2e-tweaks-part-005

matticbot · 2021-03-09T17:51:41Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

WunderBart · 2021-03-11T14:21:49Z

test/e2e/lib/driver-helper.js

@@ -530,7 +529,7 @@ export async function selectElementByText( driver, selector, text ) {
 		const allElements = await driver.findElements( selector );
 		return await webdriver.promise.filter( allElements, getInnerTextMatcherFunction( text ) );
 	};
-	return await this.clickWhenClickable( driver, element, null, `while looking for '${ text }'` );
+	return await this.clickWhenClickable( driver, element );


I'm removing this only temporarily. Already working on a refactor that would include displaying similar message in #50944.

WunderBart · 2021-03-12T13:28:32Z

There's one failing flaky spec that can be ignored. It's being addressed in #50999.

fullofcaffeine · 2021-03-17T02:02:20Z

test/e2e/lib/driver-helper.js

+	 * @param {By} locator The element's locator
+	 * @returns {WebElementCondition} The new condition
+	 */
+	elementIsClickable( locator ) {


Neat way to logically group this as a local extension based on the until object 👍🏻

fullofcaffeine · 2021-03-17T02:11:52Z

test/e2e/lib/driver-helper.js

+						.getAttribute( 'aria-disabled' )
+						.then( ( v ) => v !== 'true' );
+
+					return isEnabled && isAriaEnabled ? element : null;


What if / could it happen that the element isEnabled but not isAriaEnabled? The && here seems to suggest that it might be possible.

Good question, because AFAIK it's how we actually disable elements in Gutenberg (most probably because of the inconsistency issue between screen readers). So when an element is enabled, but at the same time it's aria-disabled="true" (i.e. the Publish button) it should be treated just like a disabled element would, which in the result should be a non-clickable/non-actionable element.

fullofcaffeine · 2021-03-17T02:17:28Z

helper, where the findElement method was not called via await and, as a result, returning an unreliable state of the menu. This PR fixes that.

Out of curiosity, it'd be great if you could elaborate on why it was failing as a result of being called without an await, as it's not entirely clear to me.

fullofcaffeine

Code looks great! I see that the remaining failed Desktop E2E test is related to the loginc magic link, which is being addressed in another PR(s). 🚢

WunderBart · 2021-03-17T10:10:06Z

helper, where the findElement method was not called via await and, as a result, returning an unreliable state of the menu. This PR fixes that.

Out of curiosity, it'd be great if you could elaborate on why it was failing as a result of being called without an await, as it's not entirely clear to me.

Of course! The driver.findElement method returns a promise, so we need to either return it in the chain or await for it before it's resolved. The same goes for clickWhenClickable. We weren't doing that (#1, #2), so the actual result of the promise was not returned by that helper in time. Does that make sense?

WunderBart added the [Status] Needs e2e Testing label Mar 9, 2021

WunderBart self-assigned this Mar 9, 2021

WunderBart force-pushed the refactor/e2e-tweaks-part-005 branch from 62d905a to 6dc6043 Compare March 9, 2021 17:47

WunderBart changed the title ~~E2E: DriverHelper tweaks part 1~~ E2E: Refactor clickWhenClickable() helper Mar 10, 2021

WunderBart force-pushed the refactor/e2e-tweaks-part-005 branch from 0cf9343 to f64694a Compare March 11, 2021 13:30

WunderBart added the [Status] In Progress label Mar 11, 2021

WunderBart commented Mar 11, 2021

View reviewed changes

WunderBart marked this pull request as ready for review March 12, 2021 13:28

WunderBart added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Mar 12, 2021

WunderBart requested review from a team, worldomonation, cathymcbride and bsessions85 March 12, 2021 13:29

WunderBart added a commit that referenced this pull request Mar 12, 2021

Include changes from #50911

e77bfd6

WunderBart added a commit that referenced this pull request Mar 16, 2021

Include changes from #50911

0c5615e

WunderBart added 8 commits March 16, 2021 19:02

Ensure clickWhenClickable waits for the element to be always clickable

be9c14a

Fix 'timeout must be a number' error

23aafaa

Do not wait for element to be visible & handle error msg better

14dd18d

Move highlight before checking if element is enabled

22125f2

Fix 'element not interactable' error when navigating mobile menu

7f33b3d

Do not ensureMobileMenuOpen when BROWSERSIZE is not 'mobile'

7b7c057

Clean up and add some JSDocs

e24bb64

Extract clickability check to the elementIsClickable 'until' helper

c8cebcd

WunderBart force-pushed the refactor/e2e-tweaks-part-005 branch from 12300be to c8cebcd Compare March 16, 2021 18:04

fullofcaffeine reviewed Mar 17, 2021

View reviewed changes

fullofcaffeine approved these changes Mar 17, 2021

View reviewed changes

WunderBart added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 17, 2021

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 17, 2021

WunderBart removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 17, 2021

WunderBart merged commit 0c8c8f2 into trunk Mar 17, 2021

WunderBart deleted the refactor/e2e-tweaks-part-005 branch March 17, 2021 11:14

WunderBart removed [Status] Needs e2e Testing [Status] Ready to Merge labels Mar 17, 2021

WunderBart mentioned this pull request Jun 17, 2021

Playwright: migrate wp-media-edit-spec with additional tests #53724

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

E2E: Refactor clickWhenClickable() helper #50911

E2E: Refactor clickWhenClickable() helper #50911

WunderBart commented Mar 9, 2021 •

edited

matticbot commented Mar 9, 2021

matticbot commented Mar 9, 2021

WunderBart Mar 11, 2021

WunderBart commented Mar 12, 2021 •

edited

fullofcaffeine Mar 17, 2021 •

edited

fullofcaffeine Mar 17, 2021

WunderBart Mar 17, 2021 •

edited

fullofcaffeine commented Mar 17, 2021 •

edited

fullofcaffeine left a comment

WunderBart commented Mar 17, 2021 •

edited

E2E: Refactor clickWhenClickable() helper #50911

E2E: Refactor clickWhenClickable() helper #50911

Conversation

WunderBart commented Mar 9, 2021 • edited

Changes proposed in this pull request

Testing instructions

matticbot commented Mar 9, 2021

matticbot commented Mar 9, 2021

WunderBart Mar 11, 2021

Choose a reason for hiding this comment

WunderBart commented Mar 12, 2021 • edited

fullofcaffeine Mar 17, 2021 • edited

Choose a reason for hiding this comment

fullofcaffeine Mar 17, 2021

Choose a reason for hiding this comment

WunderBart Mar 17, 2021 • edited

Choose a reason for hiding this comment

fullofcaffeine commented Mar 17, 2021 • edited

fullofcaffeine left a comment

Choose a reason for hiding this comment

WunderBart commented Mar 17, 2021 • edited

WunderBart commented Mar 9, 2021 •

edited

WunderBart commented Mar 12, 2021 •

edited

fullofcaffeine Mar 17, 2021 •

edited

WunderBart Mar 17, 2021 •

edited

fullofcaffeine commented Mar 17, 2021 •

edited

WunderBart commented Mar 17, 2021 •

edited