Running an integration test using E2E and Flutter Driver for testing flutter web engine #49833

nturgut · 2020-01-30T22:09:17Z

Description

Replace this paragraph with a description of what this PR is doing. If you're modifying existing behavior, describe the existing behavior, how this PR is changing it, and what motivated the change. If you're changing visual properties, consider including before/after screenshots (and runnable code snippets to reproduce them).

Related Issues

Replace this paragraph with a list of issues related to this PR from our issue database. Indicate, which of these issues are resolved or fixed by this PR. There should be at least one issue listed here.

Tests

I added the following tests:

Replace this with a list of the tests that you added as part of this PR. A change in behaviour with no test covering it
will likely get reverted accidentally sooner or later. PRs must include tests for all changed/updated/fixed behaviors. See Test Coverage.

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.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: Replace with a link to your migration guide

…side. adding 2 screenshots tests to e2e tests. normal screenshot test example with chrome to hello_world_test.dart

collinjackson

Thanks, this looks promising!

Would it make sense to do the E2E binding as a PR on the package?https://github.com/flutter/plugins/tree/master/packages/e2e

Or do you think this class should be landed in the framework repo?

The text input binding changes require some cleanup but I think those are still in progress.

collinjackson · 2020-02-04T18:23:46Z

examples/hello_world/lib/e2e.dart

+
+  final Mutex screenshotLock = new Mutex();
+
+  Future<void> takeScreenshot(String testName) async {


Since this is a public API, it should be documented.

I'm wondering if we should automatically use the currently active group and test name if the developer doesn't provide a string. This might make tests more terse.

collinjackson · 2020-02-04T18:25:28Z

examples/hello_world/lib/e2e.dart

+
+  static Map<String, String> _results = <String, String>{};
+
+  // Emulates the Flutter driver extension, returning 'pass' or 'fail'.


Need to update this comment (perhaps the API can be unit tested?)

collinjackson · 2020-02-04T18:26:31Z

examples/hello_world/lib/e2e.dart

+    print('exception $exception');
+  }
+
+  /// For flutter web tests we want to use the real messages instead of


Suggested change

/// For flutter web tests we want to use the real messages instead of

/// For Flutter web tests we want to use the real messages instead of

collinjackson · 2020-02-04T18:27:51Z

examples/hello_world/lib/e2e.dart

+
+  /// For flutter web tests we want to use the real messages instead of
+  /// test messages.
+  /// See [TestDefaultBinaryMessenger].


Should have a blank line, then "See also:", then another blank line, then a bulleted list.

Example:

flutter/packages/flutter/lib/src/cupertino/icons.dart

Line 24 in 5005824

/// See also:

collinjackson · 2020-02-04T18:31:12Z

examples/hello_world/lib/e2e.dart

+    }
+
+    if (kIsWeb) {
+      print('****************** kisweb');


collinjackson · 2020-02-04T18:32:26Z

examples/hello_world/lib/_extension_web.dart

+/// Registers Web Service Extension for Flutter Web application.
+///
+/// window.$flutterDriver will be called by Flutter Web Driver to process
+/// Flutter Command.


Suggested change

/// Flutter Command.

/// Flutter command.

collinjackson · 2020-02-04T18:36:41Z

examples/hello_world/test_driver/app_e2e.dart

+    binding.framePolicy = LiveTestWidgetsFlutterBindingFramePolicy.fullyLive;
+  });
+
+  // Failing tests always halt. They timeout after 12 minutes and fail.


Tests shouldn't have to time out to fail. In the future, await tester.pumpAndSettle() will be unnecessary.

See http://flutter.dev/go/synchronized-widgettester

collinjackson · 2020-02-04T18:38:15Z

examples/hello_world/test_driver/app_e2e.dart

+    await binding.takeScreenshot('focusedInput');
+  });
+
+  // For some reason we need to explicity call complete with flutter for web


This seems worth addressing before landing

collinjackson · 2020-02-04T18:56:08Z

examples/hello_world/lib/e2e.dart

+  Completer<String> screenshotPipe = Completer<String>();
+  Completer<bool> waitingForDriverScreenshot = Completer<bool>();
+
+  final Mutex screenshotLock = new Mutex();


should be private

collinjackson · 2020-02-04T18:56:19Z

examples/hello_world/lib/e2e.dart

+
+  final Completer<bool> allTestsPassed = Completer<bool>();
+  Completer<String> screenshotPipe = Completer<String>();
+  Completer<bool> waitingForDriverScreenshot = Completer<bool>();


should be private

nturgut · 2020-02-04T20:35:16Z

Thanks, this looks promising!

Would it make sense to do the E2E binding as a PR on the package?https://github.com/flutter/plugins/tree/master/packages/e2e

Or do you think this class should be landed in the framework repo?

The text input binding changes require some cleanup but I think those are still in progress.
The PR is to show a running demo :)

Yes I will send changes to the e2e package.

goderbauer · 2020-02-19T22:44:28Z

(triage) @nturgut Are you still working on this one? It has been marked as WIP, but there haven't been any updates recently.

nturgut · 2020-02-20T17:23:57Z

(triage) @nturgut Are you still working on this one? It has been marked as WIP, but there haven't been any updates recently.

When I tried closing draft PRs in other cases the comments became invisible. I still want to use the comments on this PR so I am keeping this one open.

Thanks,

Nurhan

collinjackson · 2020-02-22T00:26:24Z

examples/hello_world/lib/e2e.dart

+  BinaryMessenger createBinaryMessenger() {
+    if (kIsWeb) {
+      print('****************** kisweb return real binary messenger');
+      return super.createOriginalBinaryMessenger();


The TestDefaultBinaryMessenger exposes the wrapped BinaryMessenger as a public delegate property. Can we take super.createBinaryMessenger(), assert that it's a TestDefaultBinaryMessenger, and read delegate so that we don't need to change the TestWidgetsFlutterBinding API?

It's also unclear to me why we need the real binary messenger here. It seems like all TestDefaultBinaryMessenger is doing is wrapping the real messenger and providing a platformMessagesFinished API.

To be clear, what I'm proposing is:

TestDefaultBinaryMessenger messenger = super.createBinaryMessenger(); return messenger.delegate;

That said, it may be fine to use the TestDefaultBinaryMessenger since all it is doing is proxying calls to the delegate and exposing a platformMessagesFinished API. If it doesn't work, I'm curious why.

nturgut · 2020-02-27T21:23:31Z

Closing the PR not to interrupt flutter triage. The code is still in discussion.

Nurhan Turgut added 2 commits January 30, 2020 11:32

e2e flutter driver working

05855e2

make text channels work with e2e. add test for web engine

06b5ca7

nturgut added a: tests "flutter test", flutter_test, or one of our tests cla: no labels Jan 30, 2020

fluttergithubbot added d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Jan 30, 2020

googlebot added cla: yes and removed cla: no labels Jan 30, 2020

nturgut mentioned this pull request Jan 31, 2020

[web][tests] Create a test harness demo using Flutter Driver #49355

Closed

adding changes to e2e for screenshot tests. adding support to driver …

bf347e5

…side. adding 2 screenshots tests to e2e tests. normal screenshot test example with chrome to hello_world_test.dart

collinjackson self-requested a review February 4, 2020 18:28

collinjackson reviewed Feb 4, 2020

View reviewed changes

nturgut mentioned this pull request Feb 5, 2020

[e2e] e2e web changes part1: registering web extension. flutter/plugins#2502

Merged

13 tasks

collinjackson mentioned this pull request Feb 11, 2020

Remove chrome platform from flutter test #50594

Closed

Piinks added the platform-web Web applications specifically label Feb 21, 2020

collinjackson reviewed Feb 22, 2020

View reviewed changes

nturgut closed this Feb 27, 2020

nturgut mentioned this pull request Feb 28, 2020

[web] adding a test for e2e web testing. flutter/plugins#2554

Merged

13 tasks

dnfield mentioned this pull request May 20, 2020

[e2e] should support running an application in as "realistic" a mode as possible #57623

Closed

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


		final Mutex screenshotLock = new Mutex();

		Future<void> takeScreenshot(String testName) async {


		static Map<String, String> _results = <String, String>{};

		// Emulates the Flutter driver extension, returning 'pass' or 'fail'.

	/// For flutter web tests we want to use the real messages instead of
	/// For Flutter web tests we want to use the real messages instead of

Running an integration test using E2E and Flutter Driver for testing flutter web engine #49833

Running an integration test using E2E and Flutter Driver for testing flutter web engine #49833

Uh oh!

Conversation

nturgut commented Jan 30, 2020

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

collinjackson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nturgut commented Feb 4, 2020

Uh oh!

goderbauer commented Feb 19, 2020

Uh oh!

nturgut commented Feb 20, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

collinjackson Feb 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nturgut commented Feb 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

collinjackson Feb 22, 2020 •

edited

Loading