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

PageFactory enhancement. Selenium for .Net #306

Closed
wants to merge 2 commits into from
Closed

PageFactory enhancement. Selenium for .Net #306

wants to merge 2 commits into from

Conversation

TikhomirovSergey
Copy link
Contributor

These tools were implemented the perfectly similar way as it was done in Selenium Java. Now it is possible to design your own attributes/algorithms in order to decorate fields/properties by:

IFieldDecorator
ILocatorFactory
IElementLocator

Default implementations are already here. They can be extended, their virtual methods can be overridden.

Appuim project needs this in order to provide the following use case (field of page/screen object):

[FindsBy(How = How.CssSelector, Using = "relevant css")] //if there the similar browser UI as UI of a //native app
[AndroidFindBy(UIAutomator = "ui Automator locator for Android")] //if page object is going to be used against Android
[iOSFindBy(Accessibility = "accessibility for iOS")] //if page object is going to be used against 
//iOS
IWebElement targetElement;  //element is used for the mobile and browser testing

It is already present on the java-client side. If it is interesting details are below:
appium/java-client#68
appium/java-client#126
appium/java-client#140
appium/java-client#145

@TikhomirovSergey
Copy link
Contributor Author

Please look at this PR. I think it could be useful for .NET users :)

@jimevans
Copy link
Member

I have a redesign of this pull request that I believe would provide the extension points you require, but more closely adheres to the existing architecture and coding conventions. I would need to borrow some of the code (with attribution) from this pull request. Would that be an acceptable compromise?

@TikhomirovSergey
Copy link
Contributor Author

Hi @jimevans
I think it would be the acceptable compromise.
Actually I don't mind if this this development will be partially used. Main points to achieve are:

  • users should be able to use default decorating engines or implement their own attributes/engines using interfaces from Selenium / extending their default implementations.
  • I think that it would be nice to declare not only IWebElement fields/properies. Here at this PR this ability is provided. Interfaces that implemented via RemoteWebElement or its subclasses can by used. Fields/properties.

I am sorry for my English :)

@TikhomirovSergey
Copy link
Contributor Author

Maybe... Can I improve something before you will start to work on it?

@jimevans
Copy link
Member

Sure, feel free to make whatever changes you want to. In general, I'd like to do away with the "locator factory that produces only one locator" pattern, but I also don't want to force the use of reflection to create the locators. I think I have a way to accomplish that in my local branch, but I'm trying to be careful about backward compatibility.

@jimevans
Copy link
Member

I've merged a version of this pull request in 689276b. It's not merged verbatim, but does give the general extension points you've asked for. There is a custom decorator interface, IPageObjectMemberDecorator, and a new IElementLocator interface that replaces the now-obsolete IElementLocatorFactory. This implementation does not mindlessly mimic the Java implementation, which is inelegant in its misuse of the factory pattern.

I've also removed some of the complicating pieces of the pull request, like the forcing of the default locator to be settable with a TimeSpan. That's a complicating detail that is not necessary. Extenders of the base implementation are free to add that if they like.

If you have further improvements or fixes, feel free to open a new pull request, but let's use this code base as the new baseline for new features.

@jimevans jimevans closed this Mar 27, 2015
@TikhomirovSergey
Copy link
Contributor Author

Thanks @jimevans for the merging!
I have some more ideas that will be proposed later.

@TikhomirovSergey
Copy link
Contributor Author

I like it! 👍 Ok. ...And most of code that I've tried to propose to Selenium now I can realease on Appium-side using standart engines or extending them.
And other users can do the same for their own projects too.

@TikhomirovSergey
Copy link
Contributor Author

@jimevans I would like to improve something
I think that it is better to make proxiyng engines reusable. So I would like to make WebDriverObjectProxy.cs public and this, this and this methods protected not static and virtual. This way we can make DEFAULT engines reusable (when there not many things have to be changed) and avoid copy-paste in user projects (it is the huge drawback of the Java solution for Selenium that I am going to improve ;) ). So I will propose minor changes as soon as I can.

@jimevans
Copy link
Member

I'm not seeing the use case for opening up the proxy engine, but this is hardly the best place for such a discussion. Perhaps the IRC channel would be a better venue for something like a more detailed hashing out of the issues.

@TikhomirovSergey
Copy link
Contributor Author

Let it be as it is. It is cool now... I am sorry :)

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

Successfully merging this pull request may close these issues.

3 participants