Embedding testing app #10007

dnfield · 2019-07-22T19:54:58Z

This adds some scripts to help compile a very simple Dart and iOS app (android is todo, just has a script so far).

This app is meant to be suitable for Embedders to do integration testing with - it has no dependencies on the flutter_tools or framework, and so will not fail/flake based on variances in those downstream.

Unfortunately, there are a few hurdles right now to making full sense of this:

We don't build iOS on presubmit. If we can re-enable that, we could run tests against this on presubmit.
We don't currently have a good way to build both the host toolchain target and the device target as a single target, so I'm having trouble wrapping these in nice BUILD.gn scripts that would know what to do without making some really complicated dependencies.

/cc @matthew-carroll @xster @chinmaygarde @mklim @liyuqian

chinmaygarde · 2019-07-22T20:23:21Z

cc @gaaclarke

xster

LGTM. Thanks! This is amazing

xster · 2019-07-22T22:21:19Z

testing/scenario_app/.gitignore

@@ -0,0 +1,5 @@
+.packages
+pubspec.lock   # This only has local dependencies in it.


This might need some elaboration. I think every reader will end up asking.

Which part? Why we're excluding pubspec.lock, or what it means that it only has local dependencies?

xster · 2019-07-22T22:43:14Z

testing/scenario_app/README.md

+the face of changes to Dart or dart:ui that require upstream changes in the
+Flutter tooling.
+
+To add a new scenario, create a new subclass of `Scenario` and add it to the


nit: feel free to submit as is and I can move stuff around later.

But I think we'll probably want a directory structure like

testing/integration a bunch of tooling files dart/ some base dart:ui interfaces simple/ *.dart channel *.dart iOS/ startup/ code/ test/ memory/ code/ test/ scenario_1/ code/ test/ etc/...

i.e. I think there will be a lot of platform folders and not that many dart folders

xster · 2019-07-22T22:44:37Z

testing/scenario_app/README.md

+run:
+
+```bash
+./compile_ios_aot.sh ../../../out/host_profile ../../../out/ios_profile/clang_x64/


uber nit: isn't this more build_mock_framework.sh than compile_ios_aot (which is a subcomponent that happens to mostly build aot as a current implementation detail)?

Initially this did just about exactly what flutter build aot --target-platform=ios does. We could rename this if you think it's clearer - this script right now might be a bit overloaded, and ideally we'd have these captured as build rules based on the current build configuration.

Let's keep it up to date. If it does more today, let's name it so.

testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m

xster · 2019-07-22T22:50:03Z

testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m

+- (BOOL)application:(UIApplication*)application
+    didFinishLaunchingWithOptions:(NSDictionary*)launchOptions {
+  self.window = [[UIWindow alloc] initWithFrame:[[UIScreen mainScreen] bounds]];
+  self.window.rootViewController = [[FlutterViewController alloc] init];


oh, can we not check in the storyboard below then?

We could, I just wanted a simple app that didn't involve storyboards.

Ahh I see, I'll remove the file

xster · 2019-07-22T22:50:47Z

testing/scenario_app/ios/Scenarios/Scenarios/Info.plist

+	<true/>
+	<key>UIRequiredDeviceCapabilities</key>
+	<array>
+		<string>armv7</string>


the rest of this pr says arm64

This was generated by Xcode, can remove

Changing to arm64

xster · 2019-07-22T22:53:05Z

testing/scenario_app/lib/main.dart

+    ..scheduleFrame();
+  final ByteData data = ByteData(1);
+  data.setUint8(0, 1);
+  window.sendPlatformMessage('scenario_status', data, null);


This is just code comment right? Or does this do anything in this PR

This can be used on the embedding side to listen for when the Dart side is ready to get additional instructions. It's not currently used, I had planned to wire it up at one point and figured it could be done later.

mklim

LGTM!

mklim · 2019-07-22T19:57:28Z

testing/scenario_app/compile_android_aot.sh

+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+#TODO(dnfield): Get rid of this script and instead use proper build rules


I think these would be fine as Python scripts too.

Yes, that would be part of the process. If you want, I can try to refactor them to python for this PR

not sure python is better. It would be less hermetic

Python will run on Windows, and if we do end up using GN/Ninja for this it will expect the scripts to be in python

mklim · 2019-07-23T00:24:07Z

testing/scenario_app/ios/Scenarios/Scenarios/Assets.xcassets/Contents.json

+    "version" : 1,
+    "author" : "xcode"
+  }
+}


Do we need newlines here?

These are generated files. I could add a newline though

testing/scenario_app/lib/src/scenario.dart

xster · 2019-07-23T05:16:51Z

LGTM

cc @collinjackson I think this is complementary to your PR. We're trying something to split the areas under test. Though all the firebase and espresso parts still apply exactly as is.

dnfield · 2019-07-23T05:16:54Z

I've added an Android example as well. Can be split into a separate PR if desired.

xster

LGTM Android

testing/scenario_app/android/.settings/org.eclipse.buildship.core.prefs

testing/scenario_app/android/app/.settings/org.eclipse.buildship.core.prefs

testing/scenario_app/android/app/.settings/org.eclipse.jdt.core.prefs

xster · 2019-07-23T05:18:45Z

testing/scenario_app/android/app/src/main/res/drawable/ic_launcher_background.xml

+    android:height="108dp"
+    android:viewportWidth="108"
+    android:viewportHeight="108">
+    <path


did you draw something here? :D

This was created by default by Android Studio.

xster · 2019-07-23T05:19:37Z

testing/scenario_app/android/app/src/main/res/layout/activity_main.xml

+        android:layout_height="wrap_content"
+        android:theme="@style/AppTheme.AppBarOverlay">
+
+        <android.support.v7.widget.Toolbar


I think we can just keep it simple for now

Removed - these were created by AndroidStudio without me noticing

xster · 2019-07-23T05:20:27Z

testing/scenario_app/compile_android_aot.sh

+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+#TODO(dnfield): Get rid of this script and instead use proper build rules


not sure python is better. It would be less hermetic

matthew-carroll · 2019-07-23T05:35:28Z

@dnfield would you mind elaborating on the intention of the PR? Between @mklim's JUnit work and @collinjackson's e2e test work, I'm not quite clear on the objectives of this PR, specifically...

dnfield · 2019-07-23T05:42:10Z

@matthew-carroll - this would enable e2e/integration testing without reference to the flutter_tools or flutter framework packages in the flutter/flutter repo.

This will help in a few ways:

Tests can be focused just on the embedding and dart:ui, and don't include noise from the Flutter framework
Tests can be run independently of whether a given engine version is compatible with a given Flutter framework/tools version.
Engine patches that require changes downstream in Flutter can still be tested with this app

I see this as complementary to the efforts by @mklim and @collinjackson . We'll still want unit tests for some things, but this will enable integration tests that are a bit more focused and runnable.

matthew-carroll · 2019-07-23T05:44:34Z

Those benefits sound good. And this is not a replacement for Collin's work, which I believe will reference the framework from the engine? (@xster I'm remembering that correctly, right?)

dnfield · 2019-07-23T05:48:19Z

This may be an option for the e2e testing that Collin is working on, it depends on how that is structured.

If it's possible, I'd prefer to see an app like this one be used rather than one that depends on the Flutter framework - if we want to do e2e testing on the framework it would be better if it lived in the framework repo.

matthew-carroll · 2019-07-23T06:14:13Z

testing/scenario_app/README.md

+This folder contains a dart:ui application and scripts to compile it to AOT
+for exercising embedders.
+
+It intentionally has no dependencies on the Flutter framework or tooling, such


I'm curious what types of tests this is likely to facilitate. Do you already have some in mind?

As I think out loud, we wouldn't be able to test with any plugins, right?

We could verify that the embeddings send certain messages to Flutter, but we would be unable to verify any back-and-forth conversations that might occur, because the Flutter side would not really exist. Also, with something like dark mode, we can verify that the messages are sent to Flutter, but we can't truly do an e2e test that the dark mode feature, itself, is correctly implemented, because Flutter isn't there taking any actions.

What might this mean for the rendering system? For example, in the Android embedding, we have FlutterView, FlutterSurfaceView, FlutterTextureView, FlutterRenderer, FlutterRenderSurface...would all of these be excluded from significant testing in the absence of Flutter framework rendering? Or do you think there are testing strategies that effectively puts them through their paces without building out significant infrastructure?

Android now has splash capabilities, which will be used for most/all full-Flutter apps, but that functionality depends upon a message from Flutter letting us know that the 1st frame has been rendered. Without that message, the splash will never disappear. Is that going to be an issue with this testing approach?

We can send whatever messages we want to the embedding from the Dart code. We could write scenarios up that mimic the way the framework or a plugin would send messages.

Testing plugins with this setup would be a chore but might be possible.

I'm not sure how to answer the rendering system question - the current demo renders a moving color changing square. We could create more or less complicated things to render. We would not want to invest lots of time making those things look like Material widgets, and testing that's concerned about that probably belongs in the framework. but we certainly could create an Android app that uses various embedding classes to do interesting things (and write tests for them in the app) - right now, the Android app is just a barebones, dead simple implementation that mimics mostly what the vanilla Flutter app does.

dnfield · 2019-07-23T06:16:27Z

@jonahwilliams do you know why https://cirrus-ci.com/task/5164287662751744?command=test_web#L477 is failing?

matthew-carroll · 2019-07-23T06:16:29Z

testing/scenario_app/README.md

+
+## Building for Android
+
+In this folder, after building the `android_host` and `android_profile_arm64`


Is there a reason that we need to build for profile mode? Whenever I test, I like to run in debug mode so that I get the debug output to root cause failures. If we have the choice to use debug instead, can you add instructions here for that path?

Right now, the scripts are only wired up to build AOT apps.

Part of my goal here is to eventually have performance benchmarks based on engine commits, which we'd want to do AOT.

We could wire up JIT as a separt effort.

So at this point, what would the expectations be on a developer writing a test that's failing, assuming that the stack trace alone is insufficient to root cause the problem?

Right now that would be tough. We can add a jit path too though.

matthew-carroll · 2019-07-23T06:17:48Z

testing/scenario_app/README.md

+./compile_android_aot.sh ../../../out/host_profile ../../../out/android_profile_arm64/clang_x64/
+```
+
+This will produce a suitable `app.so` for building with an Android app.


Can you include instructions for what we need to do with the app.so? Ideally it would be great if instructions took us all the way through the execution of a test.

The script copies the .so file to the correct place in the Android test. You can then open that in Android studio and run the project. I've renamed app.so as well, I'll update the readme.

matthew-carroll · 2019-07-23T06:21:39Z

testing/scenario_app/android/app/src/main/AndroidManifest.xml

+        android:supportsRtl="true"
+        android:theme="@style/AppTheme">
+        <activity
+            android:name=".MainActivity"


I'm not entirely clear yet on exactly where the tests are executed in this project setup, but one thing to make sure is that we can support different Activitys for different tests. For example, if we only support one definition of a main Activity, then this project should be setup in a way that makes it trivial to have that Activity just be a list of buttons to other Activitys that run various use-cases. Or, if that can't be done, then we should find a way to allow for multiple projects to exist here. But I think if we're assuming that a singular MainActivity can exist for all such use-cases and tests, I'm pretty sure that will come up short rather quickly.

This is something we can do as we iterate on the project and create actual tests to run. For now, this is just to get something that can be run and used to verify the embedding.

testing/scenario_app/android/app/src/test/java/dev/flutter/scenarios/ExampleUnitTest.java

matthew-carroll · 2019-07-23T06:26:51Z

testing/scenario_app/lib/main.dart

+
+void main() {
+  window
+    ..onPlatformMessage = _handlePlatformMessage


It looks like we're emulating some standard Flutter behavior here. It would probably be a good idea to add comments explaining what these behaviors are, and given that we're carefully reproducing only a few behaviors, it would be helpful to know why each of these are provided and others are not.

I'm not implementing any real Flutter framework behavior here. I'm setting up a handler for platform messages as a stub for when we fill these out with more behavior/tests. I'm also creating a way to write multiple Dart based scenarios for testing that just handles register and forwarding some critical things from the Window.

But aren't you implementing these things because they are part of a standard Flutter contract that the embedding expects Flutter to fulfill?

The embedding should only care about dart:ui, not the full Flutter framework. I'm not sure which you mean in this question.

I'm setting up enough so I can draw things and notify the embedding that I'm ready. It's missing a lot that the framework does. It's also sending a message the framework doesn't.

Ok, the framework aside, I guess what I'm getting at is this. Let's imagine a developer comes into this code to see what's going on so they can add a test. They make their way into main() and they see this configuration logic. That developer may lack a lot of the contextual knowledge that all of us on this thread have right now. They may have a murky understanding of the exact boundary between the embedding and the framework, they may not be particularly familiar with Window, or even with the special area that is dart:ui.

So I'm wondering if it would be worth some expository here to provide at least an actionable level of familiarity with the hooks that are being implemented on window. We could have implemented a lot more behavior here, or we could have implemented less/no behavior here, but this seems to be a goldilocks position, and it might be worth explaining why that is. But that's just a suggestion.

matthew-carroll · 2019-07-23T06:30:05Z

testing/scenario_app/README.md

+
+This will produce a suitable `libapp.so` for building with an Android app and
+copy it (along with flutter.jar) to where Gradle will expect to find it to build
+the app in the `android/` folder. The app can be run by opening it in Android


This project will run in CI via script, right? If so, how might I execute that script locally to ensure that it builds, deploys, and succeeds with the test that I'm writing?

It's not set up to run on CI yet. I can wire that up as part of this PR if desired, but I would prefer doing it as a follow up PR. I'd be happy to sit with you tomorrow to go through the steps, or add clarifications to the README if something is missing...

If the README is updated with that info whenever its hooked up to CI, then that works for me.

liyuqian · 2019-07-23T17:52:23Z

Cool!

Even if we don't have Mac/iOS presubmit, we can still add a post-submit LUCI test to exercise your iOS scripts just to make sure those scripts won't be unexpectedly broken by someone else.

In addition to compiling the AOT targets, can the scripts or some README files provide instructions on how to run those compiled targets on an Android or iOS device? I can't wait to try the animated square on a real device, but it's not obvious to me how to do that.

For Android, it would be nice for the test being added to the presubmit before merging the change. If it takes a significantly longer time to implement than iOS, maybe just create a separate PR for Android?

Finally, I'm curious if there's plan to use this approach to test the Fuchsia runner once it's migrated to the engine repo?

Thanks again for the great work!

dnfield · 2019-07-23T17:53:32Z

@liyuqian - I've implemented the Android app in this PR, but it's getting pretty large and I'd like to wire it up to CI in a separate PR. I'll stop by your desk and we can talk about how to run it - I think the README is up tod ate now but can update it if you want.

flutter/engine@8ed5da8...ef99738 git log 8ed5da8..ef99738 --no-merges --oneline ef99738 Added a DartExecutor API for querying # of pending channel callbacks (flutter/engine#10021) a0ec528 Embedding testing app (flutter/engine#10007) 2bf1506 Disable DartLifecycleTest::ShuttingDownTheVMShutsDownAllIsolates in runtime_unittests. (flutter/engine#10064) 8edd257 Roll fuchsia/sdk/core/linux-amd64 from M5an7VPM8DiCcNcKe6J0CkAtLk8X9oMeJUqGOrZATIsC to XqtWTBni4xpYCTr7gqU7rFTuXNY1TZ_zOqBJrZM8c_kC (flutter/engine#10061) e32bdf5 Roll src/third_party/skia 00c680d2bb7c..e11dfd3da4d7 (18 commits) (flutter/engine#10062) 6603dbd Update .cirrus.yml (flutter/engine#10056) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (cbracken@google.com), and stop the roller if necessary.

dnfield added 4 commits July 20, 2019 10:31

dart portions

5a7c6e0

temp

7a6cd3d

Make it work for iOS

ec8a14f

Update doc

e97bf70

googlebot added the cla: yes label Jul 22, 2019

format

db71be5

xster reviewed Jul 22, 2019

View reviewed changes

mklim approved these changes Jul 23, 2019

View reviewed changes

dnfield added 2 commits July 22, 2019 20:05

review

cb6049a

Android sample

1e38736

xster reviewed Jul 23, 2019

View reviewed changes

remove unused files, update gitignore

14e4fd7

Merge remote-tracking branch 'upstream/master' into test_app

e2f4b31

matthew-carroll reviewed Jul 23, 2019

View reviewed changes

testing/scenario_app/android/app/src/test/java/dev/flutter/scenarios/ExampleUnitTest.java Outdated Show resolved Hide resolved

update android instructions

afe72d6

matthew-carroll reviewed Jul 23, 2019

View reviewed changes

remove sample test

1ade24a

matthew-carroll approved these changes Jul 23, 2019

View reviewed changes

Merge remote-tracking branch 'upstream/master' into test_app

05dfcec

dnfield merged commit a0ec528 into flutter:master Jul 23, 2019

engine-flutter-autoroll mentioned this pull request Jul 23, 2019

Roll engine 8ed5da8b655d..ef99738bafc0 (6 commits) flutter/flutter#36790

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 23, 2019

a0ec528 Embedding testing app (flutter/engine#10007)

3bcdff5

		@@ -0,0 +1,5 @@
		.packages
		pubspec.lock # This only has local dependencies in it.


		## Building for Android

		In this folder, after building the `android_host` and `android_profile_arm64`

Embedding testing app #10007

Embedding testing app #10007

Uh oh!

Conversation

dnfield commented Jul 22, 2019

Uh oh!

chinmaygarde commented Jul 22, 2019

Uh oh!

xster 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!

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!

mklim 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!

Uh oh!

xster commented Jul 23, 2019

Uh oh!

dnfield commented Jul 23, 2019

Uh oh!

xster left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

matthew-carroll commented Jul 23, 2019

Uh oh!

dnfield commented Jul 23, 2019

Uh oh!