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

rafe-g · 2016-03-31T19:11:38Z

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 · 2016-03-31T19:19:17Z

@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 · 2016-03-31T19:19:46Z

FYI @ericwengelking

TikhomirovSergey · 2016-03-31T19:22:57Z

@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 · 2016-03-31T19:24:40Z

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


sorry let me fix this, should be iOSPredicateString

rafe-g · 2016-03-31T22:00:05Z

@TikhomirovSergey added test and updated pr description.

ericwengelking · 2016-03-31T22:12:23Z

+
+ 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 · 2016-03-31T22:12:35Z

lgtm thanks @Rafael-Chavez !

jlipps · 2016-04-01T00:09:07Z

+ @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 · 2016-04-03T11:01:58Z

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

rafe-g · 2016-04-26T19:00:17Z

@TikhomirovSergey can we ship this?

TikhomirovSergey · 2016-04-27T09:10:32Z

@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 · 2016-04-27T15:56:23Z

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 · 2016-04-27T19:48:47Z

@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 · 2016-04-27T20:12:32Z

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 · 2016-04-27T23:39:49Z

@TikhomirovSergey isolated the test.

TikhomirovSergey · 2016-04-28T08:03:26Z

@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 · 2016-04-28T21:06:38Z

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 · 2016-04-29T03:57:28Z

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

TikhomirovSergey · 2016-04-29T10:17:40Z

@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 · 2016-04-29T11:42:07Z

@Rafael-Chavez
also please squash commits.

TikhomirovSergey · 2016-04-29T16:14:22Z

+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 · 2016-04-29T19:24:58Z

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

rafe-g · 2016-04-30T00:31:34Z

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

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

bootstraponline · 2016-04-30T00:35:18Z

@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 · 2016-04-30T01:04:57Z

Thanks @bootstraponline !!!!

TikhomirovSergey · 2016-05-01T21:18:26Z

@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 · 2016-05-01T21:31:24Z

Also #379 has been opened.

TikhomirovSergey · 2016-05-30T10:05:57Z

#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

Jonahssand 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:masterMay 1, 2016

TikhomirovSergey added a commit that referenced this pull request May 1, 2016
Merge pull request#378from 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

Uh oh!

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!

TikhomirovSergeyMar 31, 2016

Choose a reason for hiding this comment

Uh oh!

rafe-gMar 31, 2016

Choose a reason for hiding this comment

Uh oh!

rafe-g commented Mar 31, 2016

Uh oh!

ericwengelkingMar 31, 2016

Choose a reason for hiding this comment

Uh oh!

ericwengelking commented Mar 31, 2016

Uh oh!

jlippsApr 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!

TikhomirovSergeyApr 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!

TikhomirovSergeyApr 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

TikhomirovSergeyApr 29, 2016•
edited
Loading

TikhomirovSergeyApr 29, 2016•
edited
Loading