find element by nspredicate string. by rafe-g · Pull Request #352 · appium/java-client

rafe-g · Mar 31, 2016

Change list

Please provide briefly described change list which are you going to propose.
Added NSPredicate string to the java-client to support xcuitest driver nspredicate element search.

Types of changes

What types of changes are you proposing/introducing to Java client?
Put an x in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Details

Please provide more details about changes if it is necessary. If there are new features you can provide code samples which show the way they
work and possible use cases. Also you can create gists with pasted java code samples or put them here using markdown.
About markdown please read Mastering markdown and Writing on GitHub

rafe-g · Mar 31, 2016

@jlipps @Jonahss
This is to support the NSPredicate string added to xcuitesting framework driver facebookarchive/WebDriverAgent#124. This wont hurt being in master, people using the old UIA framework just wont be able to use this support.

rafe-g · Mar 31, 2016

FYI @ericwengelking

TikhomirovSergey · Mar 31, 2016

@Rafael-Chavez
It is cool! 👍
But there are some remarks:

Please improve the PR description
please add some test which show how the feature works.

TikhomirovSergey · Mar 31, 2016

src/main/java/io/appium/java_client/MobileBy.java

    }
-
+
+    public static By IosNsPredicateString(final String id) {


sorry let me fix this, should be iOSPredicateString

rafe-g · Mar 31, 2016

@TikhomirovSergey added test and updated pr description.

ericwengelking · Mar 31, 2016

src/main/java/io/appium/java_client/MobileBy.java

+
+    public static By IosNsPredicateString(final String iOSNsPredicateString) {
+    	if (iOSNsPredicateString == null) {
+    	  throw new IllegalArgumentException("Must supply an iOS NsPredicate String");


Since this is just a comment I would format it "NSPredicate" instead of "NsPredicate"

ericwengelking · Mar 31, 2016

lgtm thanks @Rafael-Chavez !

jlipps · Apr 1, 2016

src/main/java/io/appium/java_client/ios/IOSDriver.java

+    @SuppressWarnings("unchecked") @Override
+    public RequiredElementType findElementByIosNsPredicate(String using)
+    throws WebDriverException {
+        return (RequiredElementType) findElement("predicate string", using);


I'd prefer for the locator strategy to be called "-ios ns predicate" or "-ios predicate string". either way it must have a - in front of it (see "-ios uiautomation" for analogy above)

TikhomirovSergey · Apr 3, 2016

@Rafael-Chavez I'm going to check it and merge it if everything is ok.

rafe-g · Apr 26, 2016

@TikhomirovSergey can we ship this?

TikhomirovSergey · Apr 27, 2016

@Rafael-Chavez
Sorry for the late response. I totally forgot about this PR.

Ok. I think that we are not able to merge this PR because:

I'm not sure that Appium is supporting xcuitest driver. Please read roadmap.
https://github.com/appium/appium/blob/51285550eadb907e5cf90931a0c72ec7bc4a004d/ROADMAP.md#current-roadmap-projects. @jlipps Please correct me if I'm wrong. When this supporting will be here then this PR will be merged.
You've modified the test IOSDriverTest.java. This modification is invalid. CI is going to be there in near future. If CI was here now then this PR validation would be failed. Can you move your test to another class?

rafe-g · Apr 27, 2016

Appium works with appium-xcuitest-driver (I have it running locally), however it does not come with it out of the box. Which one were you referring to @TikhomirovSergey ?

Yes ill isolate the xcui driver test that i added to a XCUIDriverTest.java class.

TikhomirovSergey · Apr 27, 2016

@Rafael-Chavez
Ok. Please isolate the test and show here how do you run it. Also I'll ask a question to appium team.

bootstraponline · Apr 27, 2016

Hi, I'm in favor of this change being merged. The documentation should clearly state it's limited to the XCUITest driver because the old UIAutomation JavaScript driver also supports predicates.

Once the test is moved to XCUIDriverTest.java then I think it'll be ready for merging.

rafe-g · Apr 27, 2016

@TikhomirovSergey isolated the test.

TikhomirovSergey · Apr 28, 2016

@Rafael-Chavez
Please add a new ISOLATED test.
...And you can provide additional documentation/description if there are many details. You can put it here: https://github.com/appium/java-client/tree/master/docs. I'll add the link to WIKI.

...And then I'll merge it

rafe-g · Apr 28, 2016

Moved the test to a separate file.

@TikhomirovSergey Is this for me to create a Readme file with instructions on how to install appium-xcuitest-driver?

SrinivasanTarget · Apr 29, 2016

@Rafael-Chavez Yes, you can create one here, https://github.com/appium/java-client/tree/master/docs

TikhomirovSergey · Apr 29, 2016

@Rafael-Chavez
Well. Test is isolated but there are some more remarks. Please make it clear.
It is not necessary to use BaseIOSTest. It was added because many integration tests use the same app and the same capability set. If something is different you can implement it as a standalone test. Take a look at IOSScrollTest

TikhomirovSergey · Apr 29, 2016

@Rafael-Chavez
also please squash commits.

TikhomirovSergey · Apr 29, 2016

src/test/java/io/appium/java_client/ios/XCUIDriverTest.java

+import org.openqa.selenium.ScreenOrientation;
+import org.openqa.selenium.html5.Location;
+
+public class XCUIDriverTest extends BaseIOSTest {


Please add remove extends BaseIOSTest and add @BeforeClass/@BeforeTest and @afterclass/@after methods. They should instantiate IOSDriver with specific capabilities, I think. May be it needs some specific server flags.

This test won't work. It is using regular iOS UI Automation.

This test won't work. It is using regular iOS UI Automation.

I've read this document
https://github.com/appium/appium-xcuitest-driver#usage

But anyway BaseIOSTest uses another app. This test won't find the target element.

rafe-g · Apr 29, 2016

@TikhomirovSergey got it working with xcuitest driver. Ill push documentation then squash my commits.

rafe-g · Apr 30, 2016

@TikhomirovSergey @SrinivasanTarget according to github admin users need to squash?

https://help.github.com/articles/about-pull-request-merge-squashing/

bootstraponline · Apr 30, 2016

@TikhomirovSergey @SrinivasanTarget according to github admin users need to squash?

admins can squash for you, but it's better to do it yourself.

$ git reset --soft head~19
$ git commit --amend
$ git push -f

rafe-g · Apr 30, 2016

Thanks @bootstraponline !!!!

TikhomirovSergey · May 1, 2016

@Rafael-Chavez
Ok. I'm merging this PR.
There is #378. Code style issues have been resolved there.
https://github.com/appium/java-client/blob/master/docs/Note-for-developers.md#code-style

addition to the PR #352

TikhomirovSergey · May 1, 2016

Also #379 has been opened.

TikhomirovSergey · May 30, 2016

#398

rafe-g mentioned this pull request Mar 31, 2016

Add support to XCUITest driver appium/appium-xcuitest-driver#27

Closed

TikhomirovSergey reviewed Mar 31, 2016
View reviewed changes

ericwengelking reviewed Mar 31, 2016
View reviewed changes

jlipps reviewed Apr 1, 2016
View reviewed changes

TikhomirovSergey added the enhancement label Apr 1, 2016

TikhomirovSergey added this to the 4.0.0 milestone Apr 1, 2016

TikhomirovSergey added enhancement and removed enhancement labels Apr 8, 2016

TikhomirovSergey removed this from the 4.0.0 milestone Apr 8, 2016

TikhomirovSergey added this to the 4.0.0 milestone Apr 28, 2016

TikhomirovSergey reviewed Apr 29, 2016
View reviewed changes

Jonahss and others added 2 commits April 29, 2016 17:48

FindElementsByNsPredicate

08d7975

Merged

92474a1

TikhomirovSergey mentioned this pull request May 1, 2016

addition to the PR #352 #378

Merged

2 tasks

TikhomirovSergey merged commit 92474a1 into appium:master May 1, 2016

TikhomirovSergey added a commit that referenced this pull request May 1, 2016

Merge pull request #378 from TikhomirovSergey/rafael-chavez-xcuitesting

00d1ba7

addition to the PR #352

TikhomirovSergey mentioned this pull request May 1, 2016

The "iosNsPredicate" should be added to the @iOSFindBy annotation #379

Closed

TikhomirovSergey added the on hold label May 21, 2016

SrinivasanTarget mentioned this pull request May 21, 2016

Removed findelement by nsPredicate string #394

Merged

3 tasks

hanneshauer mentioned this pull request May 23, 2020

fix: Update "find elements using"-strings appium/WebDriverAgent#334

Closed

Search code, repositories, users, issues, pull requests...

Uh oh!

Conversation

rafe-g commented Mar 31, 2016 • edited by bootstraponline Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change list

Types of changes

Details

Uh oh!

rafe-g commented Mar 31, 2016

Uh oh!

rafe-g commented Mar 31, 2016

Uh oh!

TikhomirovSergey commented Mar 31, 2016

Uh oh!

TikhomirovSergey Mar 31, 2016

Choose a reason for hiding this comment

Uh oh!

rafe-g Mar 31, 2016

Choose a reason for hiding this comment

Uh oh!

rafe-g commented Mar 31, 2016

Uh oh!

ericwengelking Mar 31, 2016

Choose a reason for hiding this comment

Uh oh!

ericwengelking commented Mar 31, 2016

Uh oh!

jlipps Apr 1, 2016

Choose a reason for hiding this comment

Uh oh!

TikhomirovSergey commented Apr 3, 2016

Uh oh!

rafe-g commented Apr 26, 2016

Uh oh!

TikhomirovSergey commented Apr 27, 2016

Uh oh!

rafe-g commented Apr 27, 2016

Uh oh!

TikhomirovSergey commented Apr 27, 2016

Uh oh!

bootstraponline commented Apr 27, 2016

Uh oh!

rafe-g commented Apr 27, 2016

Uh oh!

TikhomirovSergey commented Apr 28, 2016

Uh oh!

rafe-g commented Apr 28, 2016

Uh oh!

SrinivasanTarget commented Apr 29, 2016

Uh oh!

TikhomirovSergey commented Apr 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TikhomirovSergey commented Apr 29, 2016

Uh oh!

TikhomirovSergey Apr 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TikhomirovSergey Apr 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rafe-g commented Apr 29, 2016

Uh oh!

rafe-g commented Apr 30, 2016

Uh oh!

bootstraponline commented Apr 30, 2016

Uh oh!

rafe-g commented Apr 30, 2016

Uh oh!

TikhomirovSergey commented May 1, 2016

Uh oh!

TikhomirovSergey commented May 1, 2016

Uh oh!

TikhomirovSergey commented May 30, 2016

Uh oh!

Reviewers

Assignees

rafe-g commented Mar 31, 2016 •

edited by bootstraponline

Loading

TikhomirovSergey commented Apr 29, 2016 •

edited

Loading

TikhomirovSergey Apr 29, 2016 •

edited

Loading

TikhomirovSergey Apr 29, 2016 •

edited

Loading