yandex-qatools / htmlelements

Html Elements is a Java framework providing easy-to-use way of interaction with web-page elements in web-page tests.
Other
271 stars 116 forks source link

Reuse selenium locators #1

Closed dmakhno closed 11 years ago

dmakhno commented 11 years ago

After some code studying, please look into ways to make following improvements

org.openqa.selenium.support.pagefactory.DefaultElementLocator was overriden by ru.yandex.qatools.htmlelements.pagefactory.DefaultElementLocator by adding findElements method.

Making assumption it was made for List, however according to: http://code.google.com/searchframe#2tHw6m3DZzo/trunk/java/client/test/org/openqa/selenium/support/pagefactory/AnnotationsTest.java&q=FindAllBy%20package:selenium%5C.googlecode%5C.com&ct=rc&cd=1&sq= It should work in base, and should be reused.

Note: not clear enough "We need to use a little bit different way of annotations handling while keeping implemented in"

If this change was so important maybe it should be implemented in selenium base.

Full reuse would be nice, because it will allow to reuse seleinum locators such as mentioned DefaultElementLocator, AjaxElementLocator or any other.

The end result can look like

@Name("Search form")
__@DefaultElementLocator__
@Block(@FindBy(xpath = "//form"))
public class SearchArrow extends HtmlElement {
    @Name("Search request input")
    @FindBy(id = "searchInput")
    private TextInput requestInput;

    @Name("Search button")
    @FindBy(className = "b-form-button__input")
    private Button searchButton;

    public TextInput getRequestInput() {
        return requestInput;
    }

    public Button getSearchButton() {
        return searchButton;
    }
}

@DefaultElementLocator can be ommited, as default. @AjaxDefaultElementLocator(120) will introduce AjaxElementLocator with timeOutInSeconds = 120 or even @CustomElementLocator( < Factory >, < factoryParams >*)

P.S. I'm first in issue list

AlexanderTolmachev commented 11 years ago

Hi, Dmytro! It's very nice to see the first issue here.

We have overriden org.openqa.selenium.support.pagefactory.DefaultElementLocator not for adding findElements() method (it is already present in the original class). The reason is that we need to handle code>@Block</code annotation which is used to specify a way of locating a block of elements and which is a class-targeted annotation. So we need to extend default mechanism of annotation handling and to use extended mechanism in DefaultElementLocator. But there are two major problems:

  1. There is no possibility for passing your own annotation handler to the original DefaultElementLocator, org.openqa.selenium.support.pagefactory.Annotations will always be used.
  2. org.openqa.selenium.support.pagefactory.Annotations class is designed to handle only field-targeted annotations and handling of class-targeted annotations can't be added by simple inheritance.

So we've done the following:

  1. We have divided org.openqa.selenium.support.pagefactory.Annotations class into two classes: AnnotationsHandler and DefaultFieldAnnotationsHandler; the first contains base functionality for annotation handling (both field-targeted and class-targeted), the second contains default functionality for handling field-targeted annotations. Such hierarchy provides possibility for creating class-targeted annotation handlers and (if needed) specific field-targeted annotation handlers.
  2. We have changed the constructor of the original DefaultElementLocator class to provide possibility for passing your own annotation handler.

And yes, this patch is to be suggested to Selenium developers. But nevertheless, the current solution is not perfect. We are going to improve it by doing the following:

  1. Make it possible to mark classes with @FindBy and @FindBys annotations, not only fields (reference to the issue).
  2. Provide possibility for passing annotation handler to other element locators of WebDriver, not only to DefaultElementLocator, which is currently used in our library.

And after we improve our patch we will suggest it to Selenium developers.

AlexanderTolmachev commented 11 years ago

And now about using other locators: yes, it would be nice to have such possibility, thank you for the proposal. But we think that it would be better to implement it by simply allowing to pass a locator to initialization methods of HtmlElementLoader class instead of adding new annotations. There are the following reasons:

What is your opinion?

I've also created separate related issues about implementation of this proposal: Make it possible to pass element locator to the initialization methods, Use AjaxElementLocator by default instead of DefaultElementLocator.

eroshenkoam commented 11 years ago

issues #4 and #3 are closed. do we need to do something else?