-
-
Notifications
You must be signed in to change notification settings - Fork 75
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
feat: Speed up xpath lookup by only including the requested attributes #637
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a preference comment, otherwise lgtm
import kotlin.test.assertEquals | ||
import kotlin.test.assertNull | ||
|
||
class `Xpath Query Parser Tests` { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, I thought we had Truth in androidx, but it seems this did not as an assertion library.
viewText?.let { | ||
setAttribute(ViewAttributesEnum.TEXT, it.rawText) | ||
setAttribute(ViewAttributesEnum.HINT, it.isHint) | ||
if (null == includedAttributes || includedAttributes.contains(ViewAttributesEnum.INDEX)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about define a variable for null == includedAttributes
?
val shouldSetAllAttributes = null == includedAttributes
if (shouldSetAllAttributes || includedAttributes.contains(ViewAttributesEnum.INDEX)) {
// ...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've refactored it in more functional style
recordAdapterViewInfo(view) | ||
var isTextOrHintRecorded = false | ||
var isAdapterInfoRecorded = false | ||
linkedMapOf( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍
Querying view attributes is expensive and takes time. Although xpath lookup only requires some attributes to be there and does not need them all