feat: native CSS selector for Android UiAutomator #410

dpgraham · 2020-09-04T03:34:36Z

The CSS selector is going unused in all Appium Drivers 😢 . Why not put it to use?

This PR adds support for a native CSS selector strategy. Instead of using CSS to select HTML elements, it parses CSS selectors and translate the selector to a native -android uiautomator selector.

Example

android.widget.TextView[description="Some description"]

translates to

new UiSelector().className("android.widget.TextView").description("Some description")

ie:

elementByCss('android.widget.TextView[description="Some description"]')

does the same thing as

elementBy('-android uiautomator', 'new UiSelector().className("android.widget.TextView").description("Some description")')

many more examples in css-converter-specs.js and by-css-e2e-specs.js

Versatile

This selector can be used to replace every other selector:

find by id:
elementsById("someResourceID") -->
elementsByCss("#someResourceID")
find by class name:
elementsByClassName("android.widget.TextView") -->
elementsByCss("android.widget.TextView")
find by accessibility id:
elementsByAccessibilityId("Some Content Description") -->
elementsByCss('*[description="Some Content Description"]')
find by xpath:
elementsByXpath("//android.widget.TextView[@description='Accessibility']") --> elementsByCss("android.widget.TextView[description='Accessibility']")

Performance

Unlike Xpath, this selector doesn't require a document to be generated that the selector is applied to. It just parses the CSS selector and translates it to an analagous UiAutomator selector.

How is it done?

css is parsed by css-converter.js which uses the https://www.npmjs.com/package/css-selector-parser library
the CSS object is parsed and converted to a UiSelector expression that is passed to UiAutomator2 Server as -android uiautomator selector

How is it different from CSS selectors being applied to HTML?

Throws exceptions if unsupported attributes are given. This is so that it's more helpful for users so they can see when they're using a bad attribute
The descendant operators (>, ~, ...) are unsupported.
Class names:
- For HTML CSS selectors, a selector like p.classOne.classTwo means that it's matching an HTML <p> element that has classOne and classTwo as class attributes (order doesn't matter).
- For this UiAutomator2 CSS selector, a selector like android.widget.TextView means that it's matching a widget with the fully qualified Java class name android.widget.TextView

Next Step

Support this in iOS with class chain and in Espresso with ViewMatchers and DataMatchers

lib/css-converter.js

mykola-mokhnach · 2020-09-04T05:27:59Z

lib/css-converter.js

+ *
+ * @param {*} str
+ */
+function escapeRegexLiterals (str) {


lodash already has a helper to escape regex

Found this: https://stackoverflow.com/questions/3561493/is-there-a-regexp-escape-function-in-javascript ... copied and pasted. I couldn't find lodash version of this.

https://lodash.com/docs/4.17.15#escapeRegExp

lib/css-converter.js

mykola-mokhnach · 2020-09-04T05:34:01Z

lib/css-converter.js

+ * Convert a CSS rule to a UiSelector
+ * @param {*} cssRule CSS rule definition
+ */
+function parseCssRule (cssRule) {


I think it makes sense to have a class for such transformation rather than a set of functions.

@mykola-mokhnach I prefer the procedural approach myself, but that's a much larger debate.

having a class could help to avoid passing the css object (and possibly many other shared args) to all the functions

also, this could be refactored to have a main class that performs css expression validation and tokenization and then passing a tranformer class/function to it. This class/function could be responsible for the expression tranformation itself to, let say, UiSelector or class chain expression. This way we could extract the common part of the code and possibly reuse it by moving to appium-support module. So, only the driver-specific transformation method would stay in the driver source.

I chose this approach because of my personal preference ... I find the testing simpler and I personally lean towards avoiding the cognitive overhead of thinking about "state". That is it to say, when I'm writing and troubleshooting functions, I prefer the functions to be "pure" so that I only have to think about the inputs (the args) and the output and that I don't have to think about the state of the object under test.

I still believe there are many cases where using classes/objects is the right approach. I won't be advocating that we port Appium to Haskell any time soon. I like that Javascript affords the choice between functional programming and object-oriented :)

It is fine to use Haskell-based approach. For me it's more important to have it properly designed from architectural perspective. E.g. that we could easily separate different entities (tokenization, validation and transformation) and then implement the feature in different drivers without copying the same parts of code here and there

For this feature, my plan is to not share any code between the drivers and just copy-paste + refactor.

Normally this is terrible coding practice, but I'm making an exception because this is based on the CSS selector standard, which isn't going to be introducing breaking changes any time soon and it's very unlikely that we'll ever need to make alterations to the code that would have to be ported to all drivers. I would rather each driver do it's own independent implementation.

lib/css-converter.js

mykola-mokhnach · 2020-09-04T05:38:33Z

test/functional/commands/find/by-css-e2e-specs.js

@@ -0,0 +1,78 @@
+import chai from 'chai';


I like the tests. It would also make sense to add a documentation to this feature

@mykola-mokhnach definitely. I'll add something to the appium docs and then probably an AppiumPro article.

I'm going to do the iOS and Android-Espresso versions of this next. The Espresso version is going to be awesome... a hybrid of datamatchers and viewmatchers.

for sure, if you want to guest write an appium pro article, i'd love to feature it

dpgraham · 2020-09-04T06:33:44Z

Suggestions implemented.

lib/css-converter.js

KazuCocoa · 2020-09-04T13:48:09Z

test/unit/css-converter-specs.js

+
+describe('css-converter.js', function () {
+  describe('simple cases', function () {
+    const simpleCases = [


lib/css-converter.js

mykola-mokhnach · 2020-09-04T17:07:57Z

lib/css-converter.js

+  } catch (e) {
+    log.errorAndThrow(`Could not parse CSS '${cssSelector}'. Reason: '${e}'`);
+  }
+  return parseCssObject(cssObj);


what if we mix these helper function into cssObj to avoid passing cssObj argument around:

function parseRule () { ...} .... // was function parseCssObject (css) function parse () { switch (this.type) { case 'rule': return this.parseRule(); ... } ... cssObj.parse = parse.bind(cssObj); cssObj.parseRule = parseRule.bind(cssObj);

No object gets passed around multiple times, they only ever get passed into one function and then the result is returned.

I don't think we're going to agree on this one 😄 ... I'd be happy to have a third Appium contributor be a tie-breaker.

I don't want to block the PR. You've done a great work and it would be a waste not to have it merged.

@jlipps Do you have any more comments or suggestions on the PR?

Thanks @mykola-mokhnach !

mykola-mokhnach · 2020-09-04T20:13:10Z

lib/css-converter.js

+  }
+
+  if (!STR_ATTRS.includes(attrName)) {
+    return;


according to the docstring this method cannot return undefined

jlipps

i wasn't able to give the code a very close look but i think overall approach is great! i guess we could end up doing the same for iOS too.

jlipps · 2020-09-04T21:43:09Z

test/functional/commands/find/by-css-e2e-specs.js

@@ -0,0 +1,78 @@
+import chai from 'chai';


for sure, if you want to guest write an appium pro article, i'd love to feature it

dpgraham · 2020-09-05T00:44:25Z

@jlipps yeah my plan is to do this for Espresso (viewmatchers + datamatchers) and then iOS (class chain).

wswebcreation · 2020-09-05T11:37:15Z

lib/css-converter.js

@@ -0,0 +1,319 @@
+import { CssSelectorParser } from 'css-selector-parser';
+import _ from 'lodash';


Can't we just do this

Suggested change

import _ from 'lodash';

import { escapeRegExp } from 'lodash';

and change the _ escapeRegExp into escapeRegExp like we normally do with imports?

wswebcreation · 2020-09-05T11:42:10Z

lib/css-converter.js

+  }
+
+  let uiAutomatorSelector = 'new UiSelector()';
+  if (cssRule.tagName && cssRule.tagName !== '*') {


Not sure how to do this differently, but I always have some issues reading multiple if|else-statements like this

mykola-mokhnach · 2020-09-05T12:10:02Z

lib/css-converter.js

+
+  // Validate that it's a supported attribute
+  if (!STR_ATTRS.includes(attrName) && !BOOLEAN_ATTRS.includes(attrName)) {
+    throw new Error(`'${attrName}' is not a support attribute. Supported attributes are ` +


'${attrName}' attribute is not supported

dpgraham added 5 commits September 3, 2020 17:02

Mobile CSS selector

d8778d2

Allow hierarchical CSS classes

50918f1

Allow hierarchical CSS classes

cabeb89

Again

6923105

Fixes

12f894c

dpgraham changed the title ~~WIP: Mobile CSS selector for Android UiAutomator~~ feat: Mobile CSS selector for Android UiAutomator Sep 4, 2020

Add text test with unicode

9077956

dpgraham changed the title ~~feat: Mobile CSS selector for Android UiAutomator~~ WIP: feat: Mobile CSS selector for Android UiAutomator Sep 4, 2020

dpgraham changed the title ~~WIP: feat: Mobile CSS selector for Android UiAutomator~~ feat: Mobile CSS selector for Android UiAutomator Sep 4, 2020

Enforce descendant operators

a376631

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

PR fixes

7cf25fb

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

dpgraham added 3 commits September 4, 2020 00:07

PR fixes

ec69ffd

Use regex escaper

9cb11e7

Use regex escaper

ff8661d

dpgraham changed the title ~~feat: Mobile CSS selector for Android UiAutomator~~ feat: native CSS selector for Android UiAutomator Sep 4, 2020

Again

234e2be

KazuCocoa reviewed Sep 4, 2020

View reviewed changes

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

lib/css-converter.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

PR fixes

f8fecce

mykola-mokhnach reviewed Sep 4, 2020

View reviewed changes

mykola-mokhnach approved these changes Sep 4, 2020

View reviewed changes

mykola-mokhnach requested a review from jlipps September 4, 2020 20:13

dpgraham added 2 commits September 4, 2020 13:18

PR fixes

53cf04b

Again

84dff53

jlipps approved these changes Sep 4, 2020

View reviewed changes

KazuCocoa approved these changes Sep 5, 2020

View reviewed changes

wswebcreation reviewed Sep 5, 2020

View reviewed changes

mykola-mokhnach reviewed Sep 5, 2020

View reviewed changes

Final PR

b0a0f02

dpgraham merged commit ac60ec3 into master Sep 5, 2020

dpgraham deleted the dpgraham-mobile-css branch September 5, 2020 22:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: native CSS selector for Android UiAutomator #410

feat: native CSS selector for Android UiAutomator #410

dpgraham commented Sep 4, 2020 •

edited

Loading

mykola-mokhnach Sep 4, 2020

dpgraham Sep 4, 2020

mykola-mokhnach Sep 4, 2020

mykola-mokhnach Sep 4, 2020

dpgraham Sep 4, 2020

mykola-mokhnach Sep 4, 2020

mykola-mokhnach Sep 4, 2020 •

edited

Loading

dpgraham Sep 4, 2020

mykola-mokhnach Sep 4, 2020 •

edited

Loading

dpgraham Sep 4, 2020 •

edited

Loading

mykola-mokhnach Sep 4, 2020

dpgraham Sep 4, 2020

dpgraham Sep 4, 2020

jlipps Sep 4, 2020

dpgraham commented Sep 4, 2020

KazuCocoa Sep 4, 2020

mykola-mokhnach Sep 4, 2020

dpgraham Sep 4, 2020

dpgraham Sep 4, 2020

mykola-mokhnach Sep 4, 2020

dpgraham Sep 4, 2020

mykola-mokhnach Sep 4, 2020

jlipps left a comment

jlipps Sep 4, 2020

dpgraham commented Sep 5, 2020

wswebcreation Sep 5, 2020

wswebcreation Sep 5, 2020

mykola-mokhnach Sep 5, 2020

		@@ -0,0 +1,319 @@
		import { CssSelectorParser } from 'css-selector-parser';
		import _ from 'lodash';

	import _ from 'lodash';
	import { escapeRegExp } from 'lodash';

feat: native CSS selector for Android UiAutomator #410

feat: native CSS selector for Android UiAutomator #410

Conversation

dpgraham commented Sep 4, 2020 • edited Loading

Example

Versatile

Performance

How is it done?

How is it different from CSS selectors being applied to HTML?

Next Step

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mykola-mokhnach Sep 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mykola-mokhnach Sep 4, 2020 • edited Loading

Choose a reason for hiding this comment

dpgraham Sep 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpgraham commented Sep 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlipps left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpgraham commented Sep 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpgraham commented Sep 4, 2020 •

edited

Loading

mykola-mokhnach Sep 4, 2020 •

edited

Loading

mykola-mokhnach Sep 4, 2020 •

edited

Loading

dpgraham Sep 4, 2020 •

edited

Loading