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

WebDriver::findField very slow, should select by id, css, then fall back on xpath #1392

Closed
codeasaurus opened this issue Sep 9, 2014 · 6 comments

Comments

@codeasaurus
Copy link

Here is some output from selenium running a simple login test that calls $I->fillField('#login-email-input', 'foo@bar.com'). Notice that this first attempt to find #login-email-input By.xpath took 10 seconds:

12:55:13.974 INFO - Executing: [find elements: By.xpath: .//*[self::input | self::textarea | self::select][not(./@type = 'submit' or ./@type = 'image' or ./@type = 'hidden')][(((./@name = '#login-email-input') or ./@id = //label[contains(normalize-space(string(.)), '#login-email-input')]/@for) or ./@placeholder = '#login-email-input')] | .//label[contains(normalize-space(string(.)), '#login-email-input')]//.//*[self::input | self::textarea | self::select][not(./@type = 'submit' or ./@type = 'image' or ./@type = 'hidden')]])

12:55:23.986 INFO - Done: [find elements: By.xpath: .//*[self::input | self::textarea | self::select][not(./@type = 'submit' or ./@type = 'image' or ./@type = 'hidden')][(((./@name = '#login-email-input') or ./@id = //label[contains(normalize-space(string(.)), '#login-email-input')]/@for) or ./@placeholder = '#login-email-input')] | .//label[contains(normalize-space(string(.)), '#login-email-input')]//.//*[self::input | self::textarea | self::select][not(./@type = 'submit' or ./@type = 'image' or ./@type = 'hidden')]]

Then the second attempt By.xpath took 11 sec:

12:55:23.989 INFO - Executing: [find elements: By.xpath: .//*[self::input | self::textarea | self::select][@name = '#login-email-input']])

12:55:34.021 INFO - Done: [find elements: By.xpath: .//*[self::input | self::textarea | self::select][@name = '#login-email-input']]

... And when it finally used find By.id, it took 2/10th of a second:

12:55:34.024 INFO - Executing: [find elements: By.id: login-email-input])

12:55:34.042 INFO - Done: [find elements: By.id: login-email-input]

Seems that finding via css selectors is far more efficient, and should thus be attempted first.

@codeasaurus
Copy link
Author

I have implemented a fix in my local code that makes running my tests tolerable. Without this, codeception is unusable.

protected function findField($selector)
{
    try
    {
        return $this->matchFirstOrFail($this->webDriver, $selector);
    }
    catch(ElementNotFound $e)
    {
        $locator = Crawler::xpathLiteral(trim($selector));

        // by text or label
        $xpath = Locator::combine(
        ...
        // all other code from previous line to next is the same
        ...
        throw new ElementNotFound($selector, "Field by name, label, CSS or XPath");
    }
}

This first attempts to find the element using matchFirstOrFail(), which much more efficiently searches by id or css, then falls back on the more expensive xpath location if it fails. Perhaps not the most elegant solution, but it works. In my case, it increases the speed of the fillField calls 100x or more

@DavertMik
Copy link
Member

@codeasaurus Good point.

I think your patch can be improved and submitted as Pull Request. At least you don't need this try/catch thing, you should better use $this->match to avoid exceptions. As you pointed, it's much effective to search by ID, CSS, before searching by semantic elements. Just a hint: this can also be done by specifying strict locator: fillField(['css' => 'form.login input'], 'Hello').

Please rewrite your code to use match method and send PR into 2.0 branch.
Thanks

@codeasaurus
Copy link
Author

Excellent! I wondered if there was a way to explicitly define the locator type, but couldn't find it in the documentation.

I will rewrite the code as you have requested and submit the PR.

Thank you!

codeasaurus added a commit to codeasaurus/Codeception that referenced this issue Sep 10, 2014
findField() was performing an efficient css search when $selector passed in as an array (e.g. ['css' => 'div.color']), but not as a string (e.g. 'div.color', '#my-id'). Fix allows efficiency when passing a string identifier by not assuming that all strings require xpath to find the element.
@codeasaurus
Copy link
Author

@DavertMik I tried doing as you suggested by passing in the strict locator array, but I do this by defining my selector in the LoginPage.php file:

public static $emailField = ['css' => '#login-email-input'];

In my test, I must wait for element visible before filling the field:

$I->waitForElementVisible(\LoginPage::$emailField, 10);
$I->fillField(\LoginPage::$emailField, $email);

Problem here is that none of the waitFor...() functions can deal with an array. I can get around this problem by defining $emailField = '#login-email-input' then:

$I->waitForElementVisible(\LoginPage::$emailField, 10);
$I->fillField(['css' => \LoginPage::$emailField], $email);

But when the methods in WebDriver are inconsistent in the types of selectors they accept, it undermines the purpose of the page file.

REF: http://codeception.com/docs/07-AdvancedUsage#PageObjects

@codeasaurus
Copy link
Author

FWIW

$I->fillField('#login-email-input', $email);

is consistently faster than:

$I->fillField(['css' => '#login-email-input'], $email);

with my fix, that is

@DavertMik
Copy link
Member

But when the methods in WebDriver are inconsistent in the types of selectors they accept, it undermines the purpose of the page file.

@codeasaurus this is what definitely should be improved. Could you create a new issue for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants