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

#574 FIX #582

Merged
merged 1 commit into from
Feb 25, 2017
Merged

#574 FIX #582

merged 1 commit into from
Feb 25, 2017

Conversation

TikhomirovSergey
Copy link
Contributor

Change list

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));
    }
}

image

@TikhomirovSergey TikhomirovSergey added this to the 5.0.0 milestone Feb 24, 2017
@TikhomirovSergey TikhomirovSergey self-assigned this Feb 24, 2017
@TikhomirovSergey
Copy link
Contributor Author

@vikramvi @SrinivasanTarget Could you review this PR.

@TikhomirovSergey
Copy link
Contributor Author

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

@saikrishna321
Copy link
Member

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

@SrinivasanTarget
Copy link
Member

cool @saikrishna321 👍

Copy link
Member

@SrinivasanTarget SrinivasanTarget left a comment

Choose a reason for hiding this comment

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

Awesome 👍

@TikhomirovSergey
Copy link
Contributor Author

ok. I'm merging.

@TikhomirovSergey TikhomirovSergey merged commit fe01575 into appium:master Feb 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants