Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

deflake Control.click and Control.selectDropdownMenu #619

Merged
merged 4 commits into from
Nov 9, 2020

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented Nov 6, 2020

fixes various flakes.

click() was flaky when the WebElement had to be scrolled into view due to obsuring by the "Save" and "Apply" buttons (so we waitfor the click to happen).

the dropDown selector was flaky and would sometimes not be visible - so also changed to using waits (and update the code that was sleeping to use wait to make the tests faster where possible)

on some systems clicking a button will fail as the button is obscured by
the "Save"/"Apply" buttons in configuration.

whilst Selenium scrolls it appears as though the click is done whilst
the scrolling is still in progress (the diagnostic screenshot shows
everything is ok) but this is taken very shortly after the test failure.

With this just try and click a few times if the button is obscured.

Ideally we would re-write the Jenkins UX so that the Save/Apply buttons
did not float above other parts of the form but rather where at the
bottom and the form contents would stop above the buttons.
but that is not such an easy fix so for now just try a few times,
on some systems the menu would fail to be displayed when we tried to
click on it as it is shown asynchronously.

with this change we wait for it to be visible, and once clicked we wait
for it to be not visible also so we know the click has done something.
@jtnord jtnord requested review from amuniz and aHenryJard November 6, 2020 14:00
WebElement we = resolve();
// button may be obscured by say the "Save Apply" screen so we wait as Selenium will do a scroll but the CSS
// can take a while to update the layout \o/
waitFor(we).
Copy link
Member Author

Choose a reason for hiding this comment

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

@aHenryJard this is the flake in the MCP tests.

I would prefer a method to actually check if something is clickable rather than trying repeatedly but I could not find anything.

@@ -186,15 +195,18 @@ public void set(Object text) {
*/
public void selectDropdownMenu(Class type) {
click();
findCaption(type,findDropDownMenuItem).click();
elasticSleep(1000);
Copy link
Member Author

@jtnord jtnord Nov 6, 2020

Choose a reason for hiding this comment

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

sleeps must die :)

the previous change (although it worked) probably worked by fluke.

repeatidly getting the element in a waitFor did not change anything
apart from taking a bit longer to do its thing (which solved the case
but for the wrong reasons).

the menu can be found regardless of if it is visible or not. so we just
find it and then wait for it to become visible.
@jtnord jtnord added the bug label Nov 6, 2020
@jtnord jtnord changed the title Click and menu flakes deflake Control.click and Control.selectDropdownMenu Nov 6, 2020
Copy link
Contributor

@aHenryJard aHenryJard left a comment

Choose a reason for hiding this comment

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

Thank you !!
I just find all timeout/waiting times hard to read, if it's possible to use a shorter way like in Duration something similar to time.asSecond(1) time.asMillisecond(1000) it would be great. (because withTimeout() and pollingEvery() can be call with only one parameter Duration, I am not sure it was clear above)

@jtnord jtnord requested a review from aHenryJard November 6, 2020 16:42
@@ -103,7 +105,14 @@ public void check(boolean state) {
* @see #clickAndWaitToBecomeStale(Duration)
*/
public void click() {
resolve().click();
WebElement we = resolve();
// button may be obscured by say the "Save Apply" screen so we wait as Selenium will do a scroll but the CSS
Copy link
Member

@amuniz amuniz Nov 6, 2020

Choose a reason for hiding this comment

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

It's not only Selenium but

public void scrollIntoView(WebElement e, WebDriver driver) {

That might be contributing to weirdness...

Copy link
Member Author

Choose a reason for hiding this comment

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

ahh thanks @amuniz I was wondering where the scrolling was happening as it didn;t seem to be Selenium :)

@amuniz
Copy link
Member

amuniz commented Nov 9, 2020

99 failures, 314 in the latest master build... so this is improving the situation! Go ahead!

@jtnord jtnord merged commit 9960239 into jenkinsci:master Nov 9, 2020
@jtnord jtnord deleted the click_and_menu_flakes branch November 9, 2020 11:02
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants