Skip to content

Add support in WidgetTester for an array of inputs#60796

Merged
CareF merged 17 commits intoflutter:masterfrom
CareF:input_event_array_test
Jul 10, 2020
Merged

Add support in WidgetTester for an array of inputs#60796
CareF merged 17 commits intoflutter:masterfrom
CareF:input_event_array_test

Conversation

@CareF
Copy link
Contributor

@CareF CareF commented Jul 3, 2020

Description

Add support to use a stream of input PointerEvents to drive a test.

The motivation of this is to enable the use of WidgetTester as a tool to drive a live integration test app on a device, with e2e or simple flutter run, for performance purposes. With this feature the long term goal is to provide a solution of host-independent test and migrating current flutter_driver based test to such solution, so that we can perform test on Firebase test lab and use the solution for shader warm up test runs. The short term goal is to have an e2e based scrolling test to catch #41118. For such purposes we need an API to drive the test app by a stream of inputs with some time duration.

Use cases would be for example drive the app similar to Scroll in flutter_driver which has a duration as parameter. Using device driven solution will do better than flutter_driver because on the device time control is more accurate than flutter_driver, and therefore input time can be controlled at sub-frame-interval level. Another demo of the use case is https://github.com/CareF/scroll_test/tree/external where I implement recording physical user inputs in one test app (by running flutter drive -t test/using_record.dart --driver test_driver/scrolling_test_e2e_test.dart -d moto --trace-startup) to generate data for another test app replaying the recorded input (by flutter drive -t test/using_record.dart --driver test_driver/scrolling_test_e2e_test.dart -d moto --trace-startup --profile). The recording part of this demo will be published as a separate package.

This was part of #60649 and #60741.

However currently our APIs in WidgetTester have very limited design for inputs that's time stamped. This is needed for the above motivation, and implementing it is the main goal of this PR. Apart from that, this also enables us to customize the input PointerEvents that can be as faithful as the real world.

To achieve this, we need:

  1. A mechanism in the test binding to advance the clock of the TestWidgetsFlutterBinding, which means for AutomatedTestWidgetsFlutterBinding to advance the clock and for LiveTestWidgetsFlutterBinding to wait a period of time. The situations when an app binds to which of these two bindings, together with E2EWidgetsFlutterBinding which is a subclass of LiveTestWidgetsFlutterBinding and _DriverBinding of flutter_driver are listed below:
command start point Binding
flutter test testWidgets(...) AutomatedTestWidgetsFlutterBinding
FLUTTER_TEST=[..] flutter test testWidgets(...) LiveTestWidgetsFlutterBinding
flutter test LiveTestWidgetsFlutterBinding();
testWidgets(...)
LiveTestWidgetsFlutterBinding
flutter run testWidgets(...) LiveTestWidgetsFlutterBinding
flutter drive enableFlutterDriverExtension() _DriverBinding
flutter drive E2EWidgetsFlutterBinding.ensureInitialized();
testWidgets(...)
E2EWidgetsFlutterBinding

FLUTTER_TEST=[..] means the environment variable FLUTTER_TEST being anything but false or empty.

  1. A method in WidgetTester that accepts an array of inputs with timestamp. Note that this timestamp is not necessarily the timestamp tagged within the event, but the timestamp for when the packet of the event is received.

These two corresponds to TestWidgetsFlutterBinding.delayed and WidgetTester.handlePointerEventPacket in this PR.

Related Issues

Resolves #60650

Tests

I added the following tests:

  • 'Input event array' in packages/flutter_test/test/widget_tester_test.dart

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.

@CareF CareF added the a: tests "flutter test", flutter_test, or one of our tests label Jul 3, 2020
@CareF CareF requested a review from liyuqian July 3, 2020 04:04
@CareF CareF self-assigned this Jul 3, 2020
@fluttergithubbot fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Jul 3, 2020
@CareF
Copy link
Contributor Author

CareF commented Jul 3, 2020

Not directly related but I think 'pumpAndSettle control test' test in packages/flutter_test/test/widget_tester_test.dart should have been put in the pumping group.

@CareF CareF changed the title Add support in WidgetTester to support an array of inputs Add support in WidgetTester for an array of inputs Jul 3, 2020
@Piinks Piinks added f: gestures flutter/packages/flutter/gestures repository. a: animation Animation APIs labels Jul 6, 2020
@CareF CareF requested a review from liyuqian July 7, 2020 12:43
@liyuqian
Copy link
Contributor

liyuqian commented Jul 7, 2020 via email

@CareF CareF requested a review from liyuqian July 7, 2020 21:10
@CareF CareF marked this pull request as draft July 7, 2020 22:29
@CareF
Copy link
Contributor Author

CareF commented Jul 10, 2020

Thanks @Hixie so much for the helpful discussion! I landed what's discussed together with some updates in the comments and redo the description of this PR for the motivation and explaining the PR. Let me know what you think about the new version of description as the document you suggested.

Another issue I discussed with @liyuqian shortly after our meeting is, probably I need to change the injected input class from PointerEvent to PointerData for enabling the integration test for #60558 . My previous trial failed on PointerData due to #60734 but I'll have a try if there's a way to get around tomorrow.

@CareF
Copy link
Contributor Author

CareF commented Jul 10, 2020

A conclusion I can draw now about preparing for #60558 is:

  1. Pointer event resampler (#41118) #60558 's solution is to put an extra layer between window.onPointerDataPacket and GestureBinding for PointerData.
  2. WidgetTester and all our test bindings are injecting PointerEvent to drive the test app, injecting PointerData with current binding will look like input from device, which is handled here:
    @override
    void dispatchEvent(PointerEvent event, HitTestResult result) {
    if (event is PointerDownEvent) {
    final RenderObject innerTarget = result.path
    .map((HitTestEntry candidate) => candidate.target)
    .whereType<RenderObject>()
    .first;
    final Element innerTargetElement = collectAllElementsFrom(
    binding.renderViewElement,
    skipOffstage: true,
    ).lastWhere(
    (Element element) => element.renderObject == innerTarget,
    orElse: () => null,
    );
    if (innerTargetElement == null) {
    debugPrint('No widgets found at ${binding.globalToLocal(event.position)}.');
    return;
    }
    final List<Element> candidates = <Element>[];
    innerTargetElement.visitAncestorElements((Element element) {
    candidates.add(element);
    return true;
    });
    assert(candidates.isNotEmpty);
    String descendantText;
    int numberOfWithTexts = 0;
    int numberOfTypes = 0;
    int totalNumber = 0;
    debugPrint('Some possible finders for the widgets at ${binding.globalToLocal(event.position)}:');
    for (final Element element in candidates) {
    if (totalNumber > 13) // an arbitrary number of finders that feels useful without being overwhelming
    break;
    totalNumber += 1; // optimistically assume we'll be able to describe it
    final Widget widget = element.widget;
    if (widget is Tooltip) {
    final Iterable<Element> matches = find.byTooltip(widget.message).evaluate();
    if (matches.length == 1) {
    debugPrint(" find.byTooltip('${widget.message}')");
    continue;
    }
    }
    if (widget is Text) {
    assert(descendantText == null);
    final Iterable<Element> matches = find.text(widget.data).evaluate();
    descendantText = widget.data;
    if (matches.length == 1) {
    debugPrint(" find.text('${widget.data}')");
    continue;
    }
    }
    final Key key = widget.key;
    if (key is ValueKey<dynamic>) {
    String keyLabel;
    if (key is ValueKey<int> ||
    key is ValueKey<double> ||
    key is ValueKey<bool>) {
    keyLabel = 'const ${key.runtimeType}(${key.value})';
    } else if (key is ValueKey<String>) {
    keyLabel = "const Key('${key.value}')";
    }
    if (keyLabel != null) {
    final Iterable<Element> matches = find.byKey(key).evaluate();
    if (matches.length == 1) {
    debugPrint(' find.byKey($keyLabel)');
    continue;
    }
    }
    }
    if (!_isPrivate(widget.runtimeType)) {
    if (numberOfTypes < 5) {
    final Iterable<Element> matches = find.byType(widget.runtimeType).evaluate();
    if (matches.length == 1) {
    debugPrint(' find.byType(${widget.runtimeType})');
    numberOfTypes += 1;
    continue;
    }
    }
    if (descendantText != null && numberOfWithTexts < 5) {
    final Iterable<Element> matches = find.widgetWithText(widget.runtimeType, descendantText).evaluate();
    if (matches.length == 1) {
    debugPrint(" find.widgetWithText(${widget.runtimeType}, '$descendantText')");
    numberOfWithTexts += 1;
    continue;
    }
    }
    }
    if (!_isPrivate(element.runtimeType)) {
    final Iterable<Element> matches = find.byElementType(element.runtimeType).evaluate();
    if (matches.length == 1) {
    debugPrint(' find.byElementType(${element.runtimeType})');
    continue;
    }
    }
    totalNumber -= 1; // if we got here, we didn't actually find something to say about it
    }
    if (totalNumber == 0)
    debugPrint(' <could not come up with any unique finders>');
    }
    }

    and will not go to widget listeners.
  3. I don't think we can integrate test Pointer event resampler (#41118) #60558 without making significant change to the binding or to write a completely new binding. But new binding is not useful for the migration of flutter_driver to a e2e based host independent test because E2EWidgetsFlutterBinding is a subclass of LiveTestWidgetsFlutterBinding.
  4. The other solution to test Pointer event resampler (#41118) #60558 (more like unit test in this case) is to have a PointerData input, push it through a stand-alone PointerDataResampler instance and convert the new PointerData to the PointerEvent, then inject it in the test app. This basically means we won't have some metric on the same benchmark test case, that can show improvement when introducing the resampling feature, like what we do for devicelab, because it's basically a different test app.
  5. The above may be more meaningful when Pointer event resampler (#41118) #60558 settles to a public API. But for now I don't think I can plan for testing this.

With the above said, I'm reopening and ask for review for the current version.

@CareF CareF closed this Jul 10, 2020
@CareF CareF reopened this Jul 10, 2020
@CareF CareF marked this pull request as ready for review July 10, 2020 04:13
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM with nits - most important one is filing the issue for the TODO :)

@CareF
Copy link
Contributor Author

CareF commented Jul 10, 2020

@dnfield Instead of filing an issue I decided to implement it. Though I don't have a very good use case for now of using this method under LiveWidgetController, the major reason being LiveWidgetController is used in a very limited situation. But in terms of the feature it's working with a unit test. Would you mind reviewing it also, or if you still insist with leaving it unimplemented, I can revert the commit.


make it another PR.

@CareF CareF merged commit a76b5eb into flutter:master Jul 10, 2020
@CareF CareF deleted the input_event_array_test branch August 7, 2020 17:50
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
* Add input event array support

* Add a tap test

* remove unused import

* remove extra assert
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: animation Animation APIs a: tests "flutter test", flutter_test, or one of our tests f: gestures flutter/packages/flutter/gestures repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using array of input events in TestWidgetsFlutterBinding as inputs

7 participants