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.

Sign up for GitHub

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

Jump to bottom

Add -android datamatcher selector strategy #386

Merged
merged 20 commits into from
Feb 12, 2019
Merged

Add -android datamatcher selector strategy #386

merged 20 commits into from
Feb 12, 2019

Conversation

dpgraham
Copy link
Contributor

@dpgraham dpgraham commented Jan 24, 2019 •
edited
Loading 

{
  name: '<name>', args: ['arg1', 'arg2', '...'], class: '<optional class>'
}

  • name: The name of a method to invoke. The method must return a Hamcrest Matcher

  • args: The args provided to the method

  • class: The class name that the method is part of (defaults to org.hamcrest.Matchers). Can be fully qualified, or simple, and simple defaults to androidx.test.espresso.matcher package (e.g.: class=CursorMatchers fully qualified is class=androidx.test.espresso.matcher.CursorMatchers

  • Uses reflection to invoke the above methods

  • Selector is like driver.elements("-android datamatcher", "{...}")

@mykola-mokhnach
Copy link
Contributor

I would be happy to see an official documentation entry for the new type of the locator, so then we could add it into client libs

@dpgraham
Copy link
Contributor Author

@mykola-mokhnach Yeah, I'm planning on doing a lot of documentation for all the new Espresso features.

@dpgraham
Copy link
Contributor Author

Fixes pushed.

@dpgraham
Copy link
Contributor Author

Just made another push. Can I merge this? (Pending Codacy and TravisCI passing, of course)

mykola-mokhnach
mykola-mokhnach reviewed Jan 28, 2019
View reviewed changes
...so-server/app/src/androidTest/java/io/appium/espressoserver/lib/helpers/GsonParserHelpers.kt
@@ -37,4 +38,26 @@ class GsonParserHelpers {
}
return defaultValue
}

fun parsePrimitive(jsonPrimitive: JsonPrimitive): Any = when {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks so Scal'ish %)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that was Kazu's idea. I like it. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://storage.googleapis.com/images.frenchly.us/2016/11/not-bad-obama.jpg

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, so Scal'ish? (I haven't worked with Scala x) )
Actually, this is kind of functional style, I think :)

mykola-mokhnach
mykola-mokhnach reviewed Jan 28, 2019
View reviewed changes
...sso-server/app/src/androidTest/java/io/appium/espressoserver/lib/helpers/KReflectionUtils.kt
@@ -53,15 +75,24 @@ object KReflectionUtils {
}
}

throw AppiumException("Could not invoke method: " +
throw AppiumException("Could not find method that matches " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the method

mykola-mokhnach
mykola-mokhnach reviewed Jan 28, 2019
View reviewed changes
...sso-server/app/src/androidTest/java/io/appium/espressoserver/lib/helpers/KReflectionUtils.kt
try {
return invokeMethod(instance::class.memberFunctions, methodName, instance, *providedParams)
} catch (e:AppiumException) {
throw AppiumException("Cannot execute method for instance of " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also use triple quotes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know how to make it exclude newlines?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trimMargin + replace, which also seem a bit redundant to me %)

mykola-mokhnach
mykola-mokhnach reviewed Jan 28, 2019
View reviewed changes
espresso-server/app/src/androidTest/java/io/appium/espressoserver/lib/model/DataMatcherJson.kt

import androidx.test.espresso.DataInteraction
import androidx.test.espresso.Espresso.onData
import com.google.gson.*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

star import?

mykola-mokhnach
mykola-mokhnach reviewed Jan 28, 2019
View reviewed changes
espresso-server/app/src/androidTest/java/io/appium/espressoserver/lib/model/DataMatcherJson.kt
val matcher = HamcrestMatcher.HamcrestMatcherDeserializer()
.deserialize(json, null, null)
.invoke();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for the empty line

mykola-mokhnach
mykola-mokhnach approved these changes Jan 28, 2019
View reviewed changes
Copy link
Contributor

@mykola-mokhnach mykola-mokhnach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some minor comments left

@appium appium deleted a comment Jan 29, 2019
@appium appium deleted a comment Jan 29, 2019
@appium appium deleted a comment Jan 29, 2019
@appium appium deleted a comment Jan 29, 2019
@appium appium deleted a comment Jan 29, 2019
@appium appium deleted a comment from mykola-mokhnach Jan 29, 2019
@appium appium deleted a comment Jan 29, 2019
@appium appium deleted a comment Jan 29, 2019
@appium appium deleted a comment from mykola-mokhnach Jan 29, 2019
@appium appium deleted a comment Jan 29, 2019
@appium appium deleted a comment Jan 29, 2019
KazuCocoa
KazuCocoa approved these changes Feb 1, 2019
View reviewed changes
Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Kotlin :)

espresso-server/app/src/androidTest/java/io/appium/espressoserver/lib/handlers/NavigateTo.java
import io.appium.espressoserver.lib.model.Element;
import io.appium.espressoserver.lib.model.NavigateToParams;
import io.appium.espressoserver.lib.model.ScrollToPageParams;
import io.appium.espressoserver.lib.model.SetTimeParams;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 clean up

espresso-server/app/src/androidTest/java/io/appium/espressoserver/lib/model/HamcrestMatcher.kt Outdated

val args = arrayListOf<Any?>()
for (arg in listOfArgs) {
if (arg.isJsonPrimitive) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nits]
maybe, here also can write like:

when {
    arg.isJsonPrimitive -> args.add(GsonParserHelpers.parsePrimitive(arg.asJsonPrimitive))
    arg.isJsonNull -> args.add(null)
    arg.isJsonObject -> args.add(HamcrestMatcherDeserializer().deserialize(arg, null, null).invoke())
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. I like that syntax.

@dpgraham dpgraham mentioned this pull request Feb 11, 2019
Closed
@dpgraham dpgraham merged commit 0a4b32d into master Feb 12, 2019
@dpgraham dpgraham deleted the dpgraham-hamcrest-matcher branch February 12, 2019 01:34
@appium appium deleted a comment Feb 12, 2019
@KazuCocoa KazuCocoa mentioned this pull request Feb 12, 2019
Merged
@mykola-mokhnach mykola-mokhnach mentioned this pull request Feb 12, 2019
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mykola-mokhnach mykola-mokhnach mykola-mokhnach approved these changes

@KazuCocoa KazuCocoa KazuCocoa approved these changes

Assignees

@mykola-mokhnach mykola-mokhnach

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dpgraham @mykola-mokhnach @KazuCocoa