#574 FIX

[TikhomirovSergey]

Change list

• wait.until(ExpectedConditions.visibilityOf(elementName)) related issue #574 BUG FIX;
• Some codestyle improvements;

Types of changes

• No changes in production code.
• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Details

@vikramvi I could find the rootcause of the bug.
Here: https://github.com/SeleniumHQ/selenium/blob/master/java/client/src/org/openqa/selenium/support/ui/ExpectedConditions.java#L293

the toString() method is invoked. And this method is overriden by RemoteWebElement. So the searching was being invoked... twice :)))

Now this sample test passes:

package io.appium.java_client.pagefactory_tests; import io.appium.java_client.android.BaseAndroidTest; import io.appium.java_client.pagefactory.AndroidFindBy; import io.appium.java_client.pagefactory.AppiumFieldDecorator; import io.appium.java_client.pagefactory.WithTimeout; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.PageFactory; import org.openqa.selenium.support.ui.ExpectedConditions; import org.openqa.selenium.support.ui.WebDriverWait; import java.util.Calendar; import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertTrue; public class BugReproducing extends BaseAndroidTest { private static final long ACCEPTABLE_DELTA_MILLS = 1500; @WithTimeout(time = 4, unit = TimeUnit.SECONDS) @AndroidFindBy(className = "ABC") //this is invalid locator; purposely put up private WebElement textView; /**  * The setting up.  */ @Before public void setUp() throws Exception { PageFactory.initElements(new AppiumFieldDecorator(driver, 15, TimeUnit.SECONDS), this); } public boolean isElementPresent(WebElement elementName, int timeout){ try{ WebDriverWait wait = new WebDriverWait(driver, timeout); wait.until(ExpectedConditions.visibilityOf(elementName)); return true; }catch(Exception e){ return false; } } private static boolean checkTimeDifference(long expectedTime, TimeUnit expectedTimeUnit, long currentMillis) { long expectedMillis = TimeUnit.MILLISECONDS.convert(expectedTime, expectedTimeUnit); try { Assert.assertEquals(true, ((currentMillis - expectedMillis) < ACCEPTABLE_DELTA_MILLS) && ( (currentMillis - expectedMillis) >= 0)); } catch (Error e) { String message = String.valueOf(expectedTime) + " " + expectedTimeUnit.toString() + " current duration in millis " + String.valueOf(currentMillis) + " Failed"; throw new AssertionError(message, e); } return true; } @Test public void findByElementTest() { long startMark = Calendar.getInstance().getTimeInMillis(); isElementPresent(textView,2); /*try {  textView.isDisplayed();  }  catch (Exception e) {  e.printStackTrace();  }*/ long endMark = Calendar.getInstance().getTimeInMillis(); assertTrue(checkTimeDifference(4, TimeUnit.SECONDS, endMark - startMark)); } }

[TikhomirovSergey]

@vikramvi @SrinivasanTarget Could you review this PR.

[TikhomirovSergey]

...I should say that the similar bug is contained by common Selenium tools.

[saikrishna321]

Non invited PR review 😉 looks good to me.. Tested and works fine 👍

[SrinivasanTarget]

cool @saikrishna321 👍

[SrinivasanTarget]

Awesome 👍

[TikhomirovSergey]

ok. I'm merging.