[+] Add support for selendroid locators in page object annotations

[clicman]

Since UiAutomator and Selendroid requires different locators I`ve added new annotations:

• SelendroidFindBy
• SelendroidFindBys
• SelendroidFindAll

And added dependency of AUTOMATION_NAME capatibility.
So now is possible to create one page object which covers all andoid versions.

[TikhomirovSergey]

As for me it is goog. :) @Jonahss What is your opinion?

But there some remarks:

• @clicman Can you add some tests here?
• Why these pairs are illegal?

checkDisallowedAnnotationPairs(androidBy, androidBys);
checkDisallowedAnnotationPairs(androidBy, selendroidBys);
checkDisallowedAnnotationPairs(androidBy, androidFindAll);
checkDisallowedAnnotationPairs(androidBy, selendroidFindAll);
checkDisallowedAnnotationPairs(androidBys, androidFindAll);
checkDisallowedAnnotationPairs(androidBys, selendroidFindAll);
...

@Jonahss
As I know the default AUTOMATION_NAME is "Appium". Am I right? So if "Selendroid" is set up at capabilities then @AndroidFindBy can be ignored.

So if I am right this checking should be fixed and it will look like this:

checkDisallowedAnnotationPairs(androidBy, androidBys);
checkDisallowedAnnotationPairs(androidBy, androidFindAll);
checkDisallowedAnnotationPairs(androidBys, androidFindAll);

checkDisallowedAnnotationPairs(iOSBy, iOSBys);
checkDisallowedAnnotationPairs(iOSBy, iOSFindAll);
checkDisallowedAnnotationPairs(iOSBys, iOSFindAll);

checkDisallowedAnnotationPairs(SelendroidFindBy, SelendroidFindBys);
checkDisallowedAnnotationPairs(SelendroidFindBys, SelendroidFindAll);
checkDisallowedAnnotationPairs(SelendroidFindBy, SelendroidFindAll);

[Jonahss]

If it's good by @TikhomirovSergey then it's good by me :)

Thanks for the Pr @clicman, can you address the comments above?

[TikhomirovSergey]

There is one more remark.
@clicman are you sure that uiAutomator and accessibility are valid strategies for Selendroid? I am not. If actually they are invalid then this and this should be removed. So the valid strategy list will be:

String id() default "";
String name() default "";
String className() default "";
String tagName() default "";
String xpath() default ""

[clicman]

@TikhomirovSergey you right uiAutomator and accessibility needs to be removed (copy-paste issue).

@Jonahss about validations: Ive added additional validations to prevent symantical errors. selendroid and android annotations are depend each other. It should not to be possible add to one page objects field @AndroidFindBy and @SelendroidFindAll annotations. In this case strong typization is not possible for this field.

[clicman]

@TikhomirovSergey one more issue about selendroid mode. In this mode you still can use @AndroidFindBy annotations. It is trick if you using finding by name, it this case you may not use selendroid annotations and minimize code. But if you in addition annotate field with selendroid - android annotation will be not used.

[TikhomirovSergey]

Ok! :) Let me show my point of view.

So, I think there are possible situations when we should run test against old Android (it means API level which is lower than 18) where UI Automator doesn't work. As I know Selendroid works bad against new API's. I can be wrong because actually I usually use automator strategies and I work with API level > = 18, if it is so please correct me.

Ok!

Here are two examples. Lets imagine that capabilities are received from parameters (test is parameterized).

So:

@BeforepublicvoidsetUp() throwsException {
Fileapp= newFile("path to your apk");
DesiredCapabilitiescapabilities = newDesiredCapabilities();
capabilities.setCapability(MobileCapabilityType.DEVICE_NAME, "Android Emulator");
capabilities.setCapability(MobileCapabilityType.APP, app.getAbsolutePath());
capabilities.setCapability(MobileCapabilityType.AUTOMATION_NAME, "Selendroid");//<===!!!capabilities.setCapability(MobileCapabilityType.PLATFORM_VERSION, "4.1");//is not necessarydriver = newAndroidDriver(newURL("http://127.0.0.1:4723/wd/hub"), capabilities);
PageFactory.initElements(newAppiumFieldDecorator(driver, 5, TimeUnit.SECONDS), desiredPageObjectInstace);
}

and

@BeforepublicvoidsetUp() throwsException {
Fileapp= newFile("path to your apk");
DesiredCapabilitiescapabilities = newDesiredCapabilities();
capabilities.setCapability(MobileCapabilityType.DEVICE_NAME, "Android Emulator");
capabilities.setCapability(MobileCapabilityType.APP, app.getAbsolutePath());
//there is no selendroid driver = newAndroidDriver(newURL("http://127.0.0.1:4723/wd/hub"), capabilities);
PageFactory.initElements(newAppiumFieldDecorator(driver, 5, TimeUnit.SECONDS), desiredPageObjectInstace);
}

The desired page/screen object looks like:

....
@SelendroidFindBy(relevantlocatorstrategyforselendroid) //this will be used when Selendroid// mode is set up at capabilities. @AndroidFindBy is ignored @AndroidFindBy(relevantlocatorstrategyforUIautomator) //this will be used when Selendroid// mode is not(!!!) set up at capabilities. @SelendroidFindBy is ignored @iOSFindBy(relevantlocatorstrategyforiOSautomation) //lets imagine that we need to test it//on iOS. It is applied when iOS platform is set up at capabilities. Android locators are ignored.privateWebElementyourElement;
....

I think it looks cool! Locator strategies are splitted for Selendroid and UI Automator modes, but all Android is covered.

@Jonahss and @clicman What are you think about this?

[TikhomirovSergey]

So
Let read the code that is proposed by @clicman. I am commenting it step by step. Ok:

AppiumAnnotations(Fieldfield, Stringplatform, Stringautomation//automation has been got from capabilities
) {
super(field);
mobileField = field;
this.platform = String.valueOf(platform).
toUpperCase().trim();
this.automation = String.valueOf(automation).
toUpperCase().trim();
} ....
@OverridepublicBybuildBy() {
assertValidAnnotations(); //firstly it validates the declaration//and here we go!!!SelendroidFindByselendroidBy = mobileField
.getAnnotation(SelendroidFindBy.class);
if (selendroidBy != null && ANDROID.toUpperCase().equals(platform) &&
"Selendroid".toUpperCase().equals(automation)) { //<=! If there is Selendroid//and field is annotated by @SelendroidFindBy returngetMobileBy(selendroidBy, //<== then it returns something relevant getFilledValue(selendroidBy)); //for Selendroid
}
//the same is for complex locator strategiesSelendroidFindBysselendroidBys = mobileField
.getAnnotation(SelendroidFindBys.class);
if (selendroidBys != null && ANDROID.toUpperCase().equals(platform) &&
"Selendroid".toUpperCase().equals(automation)) {
returngetMobileBy(selendroidBys, getFilledValue(selendroidBys)); //<=!
}
SelendroidFindAllselendroidAll = mobileField
.getAnnotation(SelendroidFindAll.class);
if (selendroidAll != null && ANDROID.toUpperCase().equals(platform) &&
"Selendroid".toUpperCase().equals(automation)) {
returngetMobileBy(selendroidAll, getFilledValue(selendroidAll)); //<=!
}
///////As you can see if now it is working at Selendroid mode @AndroidFindBy is always ignored. !!! AndroidFindByandroidBy = mobileField
.getAnnotation(AndroidFindBy.class);
if (androidBy != null && ANDROID.toUpperCase().equals(platform)) {
returngetMobileBy(androidBy, getFilledValue(androidBy));
}
AndroidFindBysandroidBys = mobileField
.getAnnotation(AndroidFindBys.class);
if (androidBys != null && ANDROID.toUpperCase().equals(platform)) {
returngetComplexMobileBy(androidBys.value(), ByChained.class);
}
AndroidFindAllandroidFindAll = mobileField.getAnnotation(AndroidFindAll.class);
if (androidFindAll != null && ANDROID.toUpperCase().equals(platform)) {
returngetComplexMobileBy(androidFindAll.value(), ByAll.class);
}
//if now there is iOS iOSFindByiOSBy = mobileField.getAnnotation(iOSFindBy.class);
if (iOSBy != null && IOS.toUpperCase().equals(platform)) {
returngetMobileBy(iOSBy, getFilledValue(iOSBy));
}
iOSFindBysiOSBys = mobileField.getAnnotation(iOSFindBys.class);
if (iOSBys != null && IOS.toUpperCase().equals(platform)) {
returngetComplexMobileBy(iOSBys.value(), ByChained.class);
}
iOSFindAlliOSFindAll = mobileField.getAnnotation(iOSFindAll.class);
if (iOSFindAll != null && IOS.toUpperCase().equals(platform)) {
returngetComplexMobileBy(iOSFindAll.value(), ByAll.class);
} //If the field is not annotated Appium-specific annotations it attempts to get By-strategy using //Selenium @FindBy'sreturnsuper.buildBy();
}

So the the addition parameter (platform) helps to rout possible Android strategies.

[TikhomirovSergey]

@Jonahss about validations: Ive added additional validations to prevent symantical errors. selendroid and android annotations are depend each other. It should not to be possible add to one page objects field @AndroidFindBy and @SelendroidFindAll annotations. In this case strong typization is not possible for this field.

I am looking at step-by-step code above and I can't find the problem :) If there were tests ( @clicman please don't forget to commit them) I think the situation would be clear.

@TikhomirovSergey one more issue about selendroid mode. In this mode you still can use @AndroidFindBy annotations. It is trick if you using finding by name, it this case you may not use selendroid annotations and minimize code. But if you in addition annotate field with selendroid - android annotation will be not used.
Ok. But id's and xpath's and something else are different. If used locators are the same on each target platform/mode it is enough to use Selenium FindBy's at all page objects in your test project.

Actually AndroidFindBy's, iOSFindBy's and SelendroidFindBy are cool when locators are different (as usually it is) on each target platform. This way we avoid the necessity to implement page objects for each mobile OS or its version.

[clicman]

@TikhomirovSergey

As I know Selendroid works bad against new API's.

I`m testing mobile apps about two weeks and has not collected enough experience to confirm or refute it. :)

I think it looks cool! Locator strategies are splitted for Selendroid and UI Automator modes, but all Android is covered.
@Jonahss and @clicman What are you think about this?

Yes its really cool. You described this pull requests behavour. Or I don`t see difference :)

I am seeing at step-by-step code above and I can't find the problem :) If there were tests ( @clicman please don't forget to commit them) I think the situation would be clear.

I`ll write tests today, but here is situation what I want to ve avoid:

@SelendroidFindBy(relevantlocatorstrategyforselendroid) //Here we suppose one WebElement object@AndroidFindAll(relevantlocatorstrategyforUIautomator) //Here we suppose List of web elementsprivateWebElementelement; // How we should typize this field in this case? Additional validations will strict such annotating.

Actually AndroidFindBy's, iOSFindBy's and SelendroidFindBy are cool when locators are different (as usually it happens) on each target platform.

Yes, but it covers this situation:

//Capatibilities are in Selendroid mode@AndroidFindBy(name="OK") //This will be used in selendroid mode if no @SelendroidFindBy annotation@IosFindBy(...)
privateWebElementokButton;
``
Codelessononestring. Justshugar, morefreedom :)

[TikhomirovSergey]

Ok! :) I think if I've described the expected behavior so why is it illegal:

@SelendroidFindBy(relevantlocatorstrategyforselendroid) //Here we suppose one WebElement object@AndroidFindAll(relevantlocatorstrategyforUIautomator) //It is actually the SINGLE element//which is found by few POSSIBLE locators. Here ByAll objects is used.privateWebElementelement; 

?

Please look at ByAll sources.

If the list is supposed here then you will implement

@SelendroidFindBy(relevantlocatorstrategyforselendroid)
@AndroidFindAll(relevantlocatorstrategyforUIautomator) privateList<WebElement> element; // !!!

Nothing is complicated!!!

There is a problem with the naming. FindAll exists in Selenium. Appium-specific annotations were named the same way. As for me the more correct is FindAny

[TikhomirovSergey]

Actually the illegal way is:

@SelendroidFindBy(relevantlocatorstrategyforselendroid)
@SelendroidFindBys(relevantlocatorstrategyforselendroid) @SelendroidFindAll(relevantlocatorstrategyforselendroid) privateWebElementelement; // or List<WebElement>

because there is not clear what strategy we should use - By, ByChained or ByAll.

So, if all is clear now I advice you to change validation from:

checkDisallowedAnnotationPairs(androidBy, androidBys);
checkDisallowedAnnotationPairs(androidBy, selendroidBys);
checkDisallowedAnnotationPairs(androidBy, androidFindAll);
checkDisallowedAnnotationPairs(androidBy, selendroidFindAll);
checkDisallowedAnnotationPairs(androidBys, androidFindAll);
checkDisallowedAnnotationPairs(androidBys, selendroidFindAll);
checkDisallowedAnnotationPairs(selendroidBy, androidBys);
checkDisallowedAnnotationPairs(selendroidBy, selendroidBys);
checkDisallowedAnnotationPairs(selendroidBy, androidFindAll);
checkDisallowedAnnotationPairs(selendroidBy, selendroidFindAll);
checkDisallowedAnnotationPairs(selendroidBys, androidFindAll);
checkDisallowedAnnotationPairs(selendroidBys, selendroidFindAll);
checkDisallowedAnnotationPairs(iOSBy, iOSBys);
checkDisallowedAnnotationPairs(iOSBy, iOSFindAll);
checkDisallowedAnnotationPairs(iOSBys, iOSFindAll);

to

checkDisallowedAnnotationPairs(androidBy, androidBys);
checkDisallowedAnnotationPairs(androidBy, androidFindAll);
checkDisallowedAnnotationPairs(androidBys, androidFindAll);
checkDisallowedAnnotationPairs(iOSBy, iOSBys);
checkDisallowedAnnotationPairs(iOSBy, iOSFindAll);
checkDisallowedAnnotationPairs(iOSBys, iOSFindAll);
checkDisallowedAnnotationPairs(SelendroidFindBy, SelendroidFindBys);
checkDisallowedAnnotationPairs(SelendroidFindBys, SelendroidFindAll);
checkDisallowedAnnotationPairs(SelendroidFindBy, SelendroidFindAll);

[clicman]

Wow! I didn`t know what ByAll is agnostic. You totally right. Thanks. I will change validations.

[clicman]

---

T E S T S

Running io.appium.java_client.pagefactory_tests.AndroidPageObjectTest
Tests run: 27, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 248.793 sec
Running io.appium.java_client.pagefactory_tests.SelendroidModeTest
Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 223.364 sec

Results :

Tests run: 39, Failures: 0, Errors: 0, Skipped: 0

Seems feature is ready to be merged. If it so, could you guys release a new version after merge?

[TikhomirovSergey]

Ok! Thank you very much! I think that @Jonahss will merge it soon.

[Jonahss]

Cool ^.^

I'll run the tests and should be able to release a new version today.

[TikhomirovSergey]

@Jonahss please wait for one day. I'll propose one more pull request. It will be bug fix + new test.

[Jonahss]

Okay :)

On Monday, December 15, 2014, Sergey Tikhomirov notifications@github.com
wrote:

@Jonahsshttps://github.com/Jonahss please wait for one day. I'll
propose one more pull request. It will be bug fix + new test.

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