Create flutter_driver infra for testing the Android AccessibilityNodeInfo generated by Flutter #19700

jonahwilliams · 2018-07-24T01:07:31Z

Forked from #19005

…egration testing

Hixie · 2018-07-24T05:53:07Z

packages/flutter_driver/lib/android_testing.dart

+// found in the LICENSE file.
+
+/// WARNING: These APIs are only supported internally for testing the flutter
+/// framework.


We either support it or we don't have it, we don't have a concept of half-public APIs.

(This is because regardless of what we say, people will depend on whatever we publish.)

Fair enough

add newline at eof

…ntics_integration_test

Hixie · 2018-08-07T02:12:00Z

packages/flutter_driver/lib/src/android/common.dart

+
+/// A Dart VM implementation of a Size.
+///
+/// Created to mirror the implementation of `Size` from 'dart:ui'


here and elsewhere: you can use [ui.Size] to get this to cross-reference correctly, I believe.

(even though it's not in scope here - dartdocs doesn't care, it puts everything in scope)

Hixie · 2018-08-07T02:12:59Z

I don't really understand how this works? There doesn't seem to be any actual logic to read anything from Android here.

Hixie · 2018-08-07T02:15:31Z

Ah, never mind, I see it in #19005.

…oid_semantics_integration_test

Hixie · 2018-08-08T23:57:18Z

I had misunderstood the purpose of this. This is for us to test our own SemanticsNode->AccessibilityNodeInfo logic. As such it probably belongs in dev/integration_tests or dev/devicelab or some such rather than flutter_driver.

…oid_semantics_integration_test

amirh

Overall looks good, left a few nits.

One more thing - right now under dev/integration_test we have a bunch of tests, I'm not sure that's the best path for this package (though the only thing that comes to mind is dev/integration_test/testing_utils and I think the team is scared of the name utils 😄 ).

I'd also consider calling a less generic name for the package, maybe something with semantics?

amirh · 2018-08-22T17:52:30Z

dev/integration_tests/android_testing/lib/src/common.dart

+
+  /// Deserializes a new [AndroidSemanticsNode] from a json map.
+  factory AndroidSemanticsNode.deserialize(String value) {
+    return new AndroidSemanticsNode._(json.decode(value));


Where is the format of this json specified?

Added a doc comment to describe the format

amirh · 2018-08-22T17:53:46Z

dev/integration_tests/android_testing/lib/src/common.dart

+  ///
+  /// This is usually only applied to text fields, which map
+  /// to "android.widget.EditText".
+  bool get isEditable => _flags['isEditable'];


I'm not sure about the json format but I guessed _flags are coming from SemanticsFlag, though I do not see any editable constant there...

These come from the AndroidAccessibilityNodeInfos. The purpose of this test is to verify that our semantics information is correctly mapped to android accessibility. I've added this to the doc comment of the class.

amirh · 2018-08-22T17:54:35Z

dev/integration_tests/android_testing/lib/src/common.dart

+  bool get isEditable => _flags['isEditable'];
+
+  /// Whether the node is enabled.
+  bool get isEnabled => _flags['isEnabled'];


Is this SemanticFlag.isEnabled?

No, see above.

amirh · 2018-08-22T17:54:52Z

dev/integration_tests/android_testing/lib/src/common.dart

+  bool get isFocusable => _flags['isFocusable'];
+
+  /// Whether the node is focused.
+  bool get isFocused => _flags['isFocused'];


No, see above.

amirh · 2018-08-22T17:54:58Z

dev/integration_tests/android_testing/lib/src/common.dart

+  bool get isEnabled => _flags['isEnabled'];
+
+  /// Whether the node is focusable.
+  bool get isFocusable => _flags['isFocusable'];


No, see above.

amirh · 2018-08-22T18:00:16Z

dev/integration_tests/android_testing/lib/src/constants.dart

+  /// accessibility action.
+  static AndroidSemanticsAction deserialize(int value) {
+    final AndroidSemanticsAction action = new AndroidSemanticsAction._(value);
+    action.toString();


Is this here to make sure value is valid?
I'd try to be more explicit about the validation here, maybe keep a static int->string map of the values, and here validate that the value is contained in the map, and in toString just query the map?

amirh · 2018-08-22T18:03:13Z

dev/integration_tests/android_testing/lib/src/matcher.dart

+///
+/// This matcher is intended to compare the accessibility values generated by
+/// the Android accessibility bridge, and not the semantics object created by
+/// the Flutter framework.


Maybe mention that unspecified/null values are not matched against

See previous sentence

amirh · 2018-08-22T18:05:43Z

dev/integration_tests/android_testing/lib/src/matcher.dart

+        return _failWithMessage('Expected actions: $actions', matchState);
+      }
+      final List<int> usedIds = <int>[];
+      outer: for (int i = 0; i < actions.length; i++) {


TIL continue outer 👍

amirh · 2018-08-22T18:07:13Z

dev/integration_tests/android_testing/lib/src/matcher.dart

+      }
+      final List<int> usedIds = <int>[];
+      outer: for (int i = 0; i < actions.length; i++) {
+        final AndroidSemanticsAction leftAction = actions[i];


nit: I was a bit confused by the name, maybe call it expected Action?

Removed by below

amirh · 2018-08-22T18:09:21Z

dev/integration_tests/android_testing/lib/src/matcher.dart

+      if (actions.length != itemActions.length) {
+        return _failWithMessage('Expected actions: $actions', matchState);
+      }
+      final List<int> usedIds = <int>[];


Is it possible to use unorderedEquals here?

RIP continue outer, Fixed!

amirh

LGTM

Just not completely sure about the package name/path

amirh · 2018-08-22T20:23:56Z

dev/integration_tests/android_testing/lib/src/common.dart

+/// A semantics node created from Android accessibility information.
+///
+/// This object represents Android accessibility information derived from an
+/// [AccessibilityNodeInfo] object. The purpose is to verify in integration


nit:
[AccessibilityNodeInfo](https://developer.android.com/reference/android/view/accessibility/AccessibilityNodeInfo)

amirh · 2018-08-22T20:24:44Z

dev/integration_tests/android_testing/lib/src/matcher.dart

+///
+/// This matcher is intended to compare the accessibility values generated by
+/// the Android accessibility bridge, and not the semantics object created by
+/// the Flutter framework.


…lityNodeInfo generated by Flutter (#19700)" This reverts commit 99b3829.

jonahwilliams added 2 commits July 23, 2018 17:32

Add infrastructure to flutter driver to perform android semantics int…

4687b3b

…egration testing

update some comments

ba9c910

googlebot added the cla: yes label Jul 24, 2018

add newline at eof

74c6f4c

Hixie reviewed Jul 24, 2018

View reviewed changes

jonahwilliams and others added 2 commits July 24, 2018 09:24

update comments and remove experimental

0cf73a6

Update constants.dart

4f78da0

jonahwilliams requested a review from goderbauer July 25, 2018 22:15

jonahwilliams added 2 commits August 1, 2018 13:28

Update matcher.dart

ab0bd3a

add newline at eof

Merge branch 'master' of github.com:flutter/flutter into android_sema…

b3c7f50

…ntics_integration_test

jonahwilliams requested review from Hixie and removed request for goderbauer August 6, 2018 20:17

Hixie reviewed Aug 7, 2018

View reviewed changes

jonahwilliams added 2 commits August 7, 2018 09:29

Merge branch 'master' of https://github.com/flutter/flutter into andr…

8f4dca5

…oid_semantics_integration_test

clean up comments

1e79e11

jonahwilliams added 8 commits August 8, 2018 18:04

Merge branch 'master' of https://github.com/flutter/flutter into andr…

8080f92

…oid_semantics_integration_test

move to devicelab projects

76cd5d3

Add tests to bot test script

b182928

Merge branch 'master' of https://github.com/flutter/flutter into andr…

f480dca

…oid_semantics_integration_test

update packages

0c07048

update to flutter test

1c33f67

Merge branch 'master' of https://github.com/flutter/flutter into andr…

4938527

…oid_semantics_integration_test

add license header and remove package test

f1b949a

amirh reviewed Aug 22, 2018

View reviewed changes

jonahwilliams added 2 commits August 22, 2018 12:58

address comments

1a37628

fix change to remove throwing from test

f678e57

amirh approved these changes Aug 22, 2018

View reviewed changes

rename package and address commits

5642b67

jonahwilliams merged commit 99b3829 into flutter:master Aug 23, 2018

jonahwilliams deleted the android_semantics_integration_test branch August 23, 2018 05:56

Hixie added a commit that referenced this pull request Aug 23, 2018

Revert "Create flutter_driver infra for testing the Android Accessibi…

ab0248c

…lityNodeInfo generated by Flutter (#19700)" This reverts commit 99b3829.

github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create flutter_driver infra for testing the Android AccessibilityNodeInfo generated by Flutter #19700

Create flutter_driver infra for testing the Android AccessibilityNodeInfo generated by Flutter #19700

jonahwilliams commented Jul 24, 2018

Hixie Jul 24, 2018

Hixie Jul 24, 2018

jonahwilliams Jul 24, 2018

Hixie Aug 7, 2018

Hixie Aug 7, 2018

Hixie commented Aug 7, 2018

Hixie commented Aug 7, 2018

Hixie commented Aug 8, 2018

amirh left a comment

amirh Aug 22, 2018

jonahwilliams Aug 22, 2018

amirh Aug 22, 2018

jonahwilliams Aug 22, 2018

amirh Aug 22, 2018

jonahwilliams Aug 22, 2018

amirh Aug 22, 2018

jonahwilliams Aug 22, 2018

amirh Aug 22, 2018

jonahwilliams Aug 22, 2018

amirh Aug 22, 2018

amirh Aug 22, 2018

jonahwilliams Aug 22, 2018

amirh Aug 22, 2018

amirh Aug 22, 2018

amirh Aug 22, 2018

jonahwilliams Aug 22, 2018

amirh Aug 22, 2018

jonahwilliams Aug 22, 2018

amirh left a comment

amirh Aug 22, 2018

amirh Aug 22, 2018

Create flutter_driver infra for testing the Android AccessibilityNodeInfo generated by Flutter #19700

Create flutter_driver infra for testing the Android AccessibilityNodeInfo generated by Flutter #19700

Conversation

jonahwilliams commented Jul 24, 2018

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

Hixie commented Aug 7, 2018

Hixie commented Aug 7, 2018

Hixie commented Aug 8, 2018

amirh left a 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

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

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

amirh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment