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

Generic AppiumDriver #178 #162 fix #182

Merged
merged 5 commits into from
Jun 19, 2015
Merged

Generic AppiumDriver #178 #162 fix #182

merged 5 commits into from
Jun 19, 2015

Conversation

TikhomirovSergey
Copy link
Contributor

The major change:

  • The ability to define the desired returned WebElement type has been provided:
AppiumDriver<MobileElement> driver;
//some actions

MobileElement e = driver.findElement(By.id("id"));
List<MobileElement> l = driver.findElements(By.id("id"));

AppiumDriver is generic now and by default its finders return WebElement (it is for backward compatibility)

Possible drawbacks and backward compatibility issues:

  • the necessity to perform casting to MobileElement is still here when there is long chain of the searching. But it is not so frequent.
MobileElement e  = driver.findElement(By.id("id"));
MobileElement<MobileElement> e2  = (MobileElement<MobileElement>) e.findElement(By.id("id2"));
MobileElement e3 = e2.findElement(By.id("id3"));
  • for Eclipse users.
    "Rawtype" warnings. This issue is resolved by the infering of a type that implements/extends WebElement or via the suppressing of warnings like that
@SuppressWarnings("rawtypes")
someClassOrFieldOfMethod
  • for contributors.
    This solution requires IDEA (Community Edition is enough) that supports Eclipse compiler. It is compatible with Eclipse Luna or higher. It is better to use Maven v3.2.5 (I'm not sure. It is possible that minimal required version is lower. Actually I just use this version.) or higher when there is necessity to perform the local building.

The built *.jar is copmatible wits both IDEs (Eclipse can be older than Luna) without special settings:

IDEA

idea

Eclipse

eclipse

Additional changes

  • declared capabilities of MobileDriver interface have been extended
  • test actualization

@Jonahss @bootstraponline
Please review this carefully. There is no need to hurry :) If these changes will be accepted I think that next version number should be 3.0

@TikhomirovSergey
Copy link
Contributor Author

#178 #162

@TikhomirovSergey
Copy link
Contributor Author

It would be cool to resolve the issue 147 before the new release...

@bootstraponline
Copy link
Member

I'll rely on @Jonahss for a detailed review. I'm not sure that the test code is any cleaner as a result of this. I see <?> everywhere. It'd be great if some users of the java-client were willing to comment since this is a large change.

@TikhomirovSergey
Copy link
Contributor Author

Ok @bootstraponline
Actually is not necessary. It is just only for demonstration that AppiumDriver and MobileElement are parameterized. With as well as without it both AppiumDriver and MobileElement work as usual

WebElement e  = driver.findElement(By.id("id"));
List<WebElement> e2  = e.findElements(By.id("id2"));

I can improve test code if it is really needed. I think it is better to replace ? by WebElement.

@Jonahss
Copy link
Member

Jonahss commented Apr 6, 2015

I'm still concerned with the need to do casting on the "chains" of finding. It's unpredictable to have to cast in that case, whereas always casting is annoying but predictable.

@TikhomirovSergey
Copy link
Contributor Author

I'm still concerned with the need to do casting on the "chains" of finding

Ok. then I will think about the way to improve this...
@Jonahss What is java_cliend will be completely migrated to

List<MobileElement> l  = e.findElements(By.id("id2"));

? As you can see it is possible. :) It is the worst case scenario. Actually I am going to look for the solution that is compatible with current user projects.

@TikhomirovSergey
Copy link
Contributor Author

It seems that I have found the acceptable solution:

  • drivers (AppiumDriver, AndroidDriver, IOSDriver) are staing generic.
  • elements (MobileElemenent, AndroidElement, IOSElement) are not generic. But their findElement* and findElements* methods return MobileElement or list of MobileElement.

So by default users will work as usual:

AppiumDriver<?> driver;
//some actions

WebElement e = driver.findElement(By.id("id"));
List<WebElement> l = driver.findElements(By.id("id"));

or

AppiumDriver<WebElement> driver;
//some actions

WebElement e = driver.findElement(By.id("id"));
List<WebElement> l = driver.findElements(By.id("id"));

But user can choose the desired type. WebElement, TouchableElement, RemoteWebElement, MobileElement or subclasses are allowed. So when they will choose something that differs from WebElement then:

AppiumDriver<MobileElement> driver;
//some actions

MobileElement e = driver.findElement(By.id("id"));
List<MobileElement> l = driver.findElements(By.id("id"));

MobileElement e1. = e.findElement(By.id("id2"));
List<MobileElement> l = e.findElements(By.id("id2"));

Everything is ready for commit. @Jonahss What do think about this? Is it ok?

@Jonahss
Copy link
Member

Jonahss commented Apr 7, 2015

Ooh yeah I like that ^.^
I'll review the code changes here and I'll update some of the examples in sample-code and such, to get a feel for it myself.

@bootstraponline
Copy link
Member

I like the changes 👍

@TikhomirovSergey
Copy link
Contributor Author

@Jonahss
I have commited improvements. Now you can check it.

When you will check this please change Java compiler to "Eclipse" if you use IDEA or download Eclipse Luna if you are Eclipse user. I advice you to perform these (for the checking) following:

  • to build snapshot and install it to your own local maven repo. It will be better if you will perform it from command line.
  • to create a small project that should depend on your local snapshot. Here you can copy/past tests or any test code you want. After please try to run tests. IDEA Java compiler can be "Javac"/ Eclipse can be Kepler or anything you want.

Actually I did the same and everything were cool. But you can do it to be sure that all is fine. Please let me know if something is wrong.

This is one of the most uncommon tasks I've ever tried to resolve :) The similar thing is going to be proposed to С#.

@Jonahss
Copy link
Member

Jonahss commented Apr 7, 2015

Haha yeah it's definitely a doozy of a Java task ;)
I'm glad the setup is still easy for our users. Only complicated for
developers on the java-client.

Someday I still think it could be nice to start fresh.

On Tue, Apr 7, 2015 at 2:46 PM, Sergey Tikhomirov notifications@github.com
wrote:

@Jonahss https://github.com/Jonahss
I have commited improvements. Now you can check it.

When you will check this please change Java compiler to "Eclipse" if you
use IDEA or download Eclipse Luna if you are Eclipse user. I advice you to
perform these (for the checking):

to build snapshot and install it to your own local maven repo. It will
be better if you will perform it from command line.

to create a small project that should depend on your local snapshot.
Here you can copy/past tests or any test code you want. After please try to
run tests. IDEA Java compiler can be "Javac"/ Eclipse can be Kepler or
anything you want.

Actually I did the same and everything were cool. But you can do it to be
sure that all is fine. Please let me know if something is wrong.

This is one of the most uncommon tasks I've ever tried to resolve :)


Reply to this email directly or view it on GitHub
#182 (comment).

@TikhomirovSergey
Copy link
Contributor Author

Soon I will provide one more commit... It is about pom.xml settings

@TikhomirovSergey
Copy link
Contributor Author

I've improved pom.xml. So let's have fun :))

in order to avoid the conflict with another PR
@TikhomirovSergey
Copy link
Contributor Author

Yeah! Required changes are on Selenium side now. (on behalf of Sergey Tikhomirov) Refactored PageFactory implementation, including a number of changes to make it more extensible and more robust.... I am going to propose a PR on this week or the beginning of the next!

@Jonahss
Copy link
Member

Jonahss commented Jun 9, 2015

Oh super awesome.
Sorry I've been so busy that I haven't looked closely at this yet. When you
submit the PR you are looking for, I should hopefully just merge it :)

On Tue, Jun 9, 2015 at 2:52 PM, Sergey Tikhomirov notifications@github.com
wrote:

Yeah! Required changes are on Selenium side now. (on behalf of Sergey
Tikhomirov) Refactored PageFactory implementation, including a number of
changes to make it more extensible and more robust....
https://raw.githubusercontent.com/SeleniumHQ/selenium/master/dotnet/CHANGELOG
I am going to propose a PR on this week or the beginning of the next!


Reply to this email directly or view it on GitHub
#182 (comment).

@TikhomirovSergey
Copy link
Contributor Author

Sorry. I have made a mistake. Another ticked has to be commented with my previous reply :))))

@Jonahss
Copy link
Member

Jonahss commented Jun 10, 2015

Oops, yeah I see it.

On Tue, Jun 9, 2015 at 3:25 PM, Sergey Tikhomirov notifications@github.com
wrote:

Sorry. I have made a mistake. Another ticked has to be commented with my
previous reply :))))


Reply to this email directly or view it on GitHub
#182 (comment).

Jonahss added a commit that referenced this pull request Jun 19, 2015
@Jonahss Jonahss merged commit 4881341 into appium:master Jun 19, 2015
@RohitBandi
Copy link

Hi All,

Can you guys help me with dot net code samples after this change with generics for AppiumDriver. I Stuck and don't know how to proceed further.

Or suggest changes for below piece of code

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using OpenQA.Selenium;
using OpenQA.Selenium.Appium;
using OpenQA.Selenium.Remote;
using OpenQA.Selenium.Appium.Android;
using OpenQA.Selenium.Support.UI;

namespace ConsoleApplication1
{
class Program
{
static void Main(string[] args)
{
DesiredCapabilities Usercapabilities;
Usercapabilities = new DesiredCapabilities();
Usercapabilities.SetCapability("deviceName", "Nexus 7");
Usercapabilities.SetCapability("platformName", "Android");

        Usercapabilities.SetCapability("platformVersion", "5.0.2");
        Usercapabilities.SetCapability(CapabilityType.BrowserName, "Chrome");

        //Application path and configurations
        AppiumDriver driver = new AppiumDriver(new Uri("http://127.0.0.1:4723/wd/hub"), Usercapabilities);
        driver.Navigate().GoToUrl("https://jobs.adp.com");
        System.Threading.Thread.Sleep(7000);
        WebDriverWait wait = new WebDriverWait(driver, TimeSpan.FromSeconds(12));
        By txt_keywordSearch = By.Name("k");
        By btn_Search = By.XPath("//button[contains(@id,'search-submit')]");
        By section_Searchresults = By.Id("search-results");
        //driver.FindElement(By.XPath("//a[contains(@data-callout-action,'job matching')]")).Click();
        if (driver.FindElement(txt_keywordSearch).Displayed)
        {
            driver.FindElement(txt_keywordSearch).SendKeys("Manager");
            driver.FindElement(btn_Search).Click();
            System.Threading.Thread.Sleep(4000);
            if (driver.FindElement(section_Searchresults).Displayed)
            {
                Console.WriteLine("successfully Navigated to search results page");
            }
            else
            {
                Console.WriteLine("Did not Navigate to search results page");
            }
        }
    }
}

}

Thanks

@Jonahss
Copy link
Member

Jonahss commented Jun 23, 2015

Could you post your issue to the dot net client repository then? this is
for java client :)

On Tue, Jun 23, 2015 at 11:09 AM, RohitBandi notifications@github.com
wrote:

Hi All,

Can you guys help me with dot net code samples after this change with
generics for AppiumDriver. I Stuck and dont know how to proceed further.

Thanks


Reply to this email directly or view it on GitHub
#182 (comment).

@fayizkc
Copy link

fayizkc commented Aug 25, 2015

is it possible to create driver object with WebDriver,
I am using the framework common for both web and mobile web, So if declare driver as AppiumDriver driver; it is not working for selenium web app. I feel this change is not good. Is it possible continue the old style itself (WebDriver) with latest Appium library.

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

Successfully merging this pull request may close these issues.

5 participants