Migrated integration_flutter_test/embedder (scenic integration test) from fuchsia.git #26880

richkadel · 2021-06-22T17:01:55Z

The tests use upgraded flutter/repo build rules, implemented in previously merged PRs, starting with #27996, to compile C++ and Dart tests as Fuchsia components, with dependencies on both C++ and Dart FIDL bindings, and on additional Dart APIs included in the Fuchsia SDK.

The tests are rewritten, from using Flutter APIs in fuchsia.git, to using only dart_ui (since flutter/engine tests cannot depend on the downstream flutter/flutter APIs), but the basic test program shells have been compiled and successfully run after publishing the test component to a running Fuchsia instance. (See //flutter/shell/platform/fuchsia/flutter/integration_flutter_tests/embedder/README.md.)

Now that the rewritten tests are complete and operational, future integration issues between Flutter and Fuchsia (e.g., Scenic) can be caught directly from flutter/engine builds, instead of being caught only from Fuchsia builds, which typically uses a version of Flutter that is at least 2 days old.

Note this PR also migrates dart_library dependencies from flutter/buildroot (secondary/third_party/... packages) to a central flutter fuchsia BUILD.gn file, so the secondary BUILD.gn files that originally defined those dart_library targets can now be removed.

List which issues are fixed by this PR. You must list at least one issue.

Fixes: flutter/flutter#90286

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.
The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

New integration tests depend on third_party Dart libraries that don't include a BUILD.gn in their upstream repositories. Add `build/secondary` BUILD.gn files (which require `flutter/tools/fuchsia/dart/dart_library.gni` defined in flutter/engine#26880) so these dart libraries can be included in Dart-based fuchsia components.

Ninja runs `action` scripts using the default python interpretter, and most flutter build scripts that use python currently require python2. In order to support python3 scripts as well, the `python3_action` invokes a wrapper python2-based script via GN `action` target, and the wrapper invokes the python3 interpretter with the given script. This new target is required to support flutter#26880, but is broken out to a separate PR (by request) to support other more immediate use cases.

New integration tests depend on third_party Dart libraries that don't include a BUILD.gn in their upstream repositories. Add `build/secondary` BUILD.gn files (which require `flutter/tools/fuchsia/dart/dart_library.gni` defined in flutter/engine#26880) so these dart libraries can be included in Dart-based fuchsia components.

Ninja runs `action` scripts using the default python interpretter, and most flutter build scripts that use python currently require python2. In order to support python3 scripts as well, the `python3_action` invokes a wrapper python2-based script via GN `action` target, and the wrapper invokes the python3 interpretter with the given script. This new target is required to support flutter#26880, but is broken out to a separate PR (by request) to support other more immediate use cases..

Ninja runs `action` scripts using the default python interpretter, and most flutter build scripts that use python currently require python2. In order to support python3 scripts as well, the `python3_action` invokes a wrapper python2-based script via GN `action` target, and the wrapper invokes the python3 interpretter with the given script. This new target is required to support flutter#26880, but is broken out to a separate PR (by request) to support other more immediate use cases.

abarth · 2021-07-08T22:35:49Z

Please do not land this pull request. It contains files from the Fuchsia repository that are not support for use outside the Fuchsia repository.

richkadel · 2021-07-08T22:43:43Z

Agreed. Thanks Adam. This Draft is not intended to be landed as-is.

Ninja runs `action` scripts using the default python interpretter, and most flutter build scripts that use python currently require python2. In order to support python3 scripts as well, the `python3_action` invokes a wrapper python2-based script via GN `action` target, and the wrapper invokes the python3 interpretter with the given script. This new target is required to support flutter#26880, but is broken out to a separate PR (by request) to support other more immediate use cases.

Ninja runs `action` scripts using the default python interpretter, and most flutter build scripts that use python currently require python2. In order to support python3 scripts as well, the `python3_action` invokes a wrapper python2-based script via GN `action` target, and the wrapper invokes the python3 interpretter with the given script. This new target is required to support #26880, but is broken out to a separate PR (by request) to support other more immediate use cases.

shell/platform/fuchsia/flutter/integration_flutter_tests/embedder/BUILD.gn

shell/platform/fuchsia/flutter/integration_flutter_tests/embedder/child-view/child_view.dart

This is a work in progress, with changes that successfully compile the embedder test. This commit leverages new build rules, added in PR flutter#27996, that enable building flutter tests as Fuchsia components, and dependencies on Dart libraries from the Fuchsia SDK (Fuchsia Dart APIs and Dart FIDL bindings). Notable: * Adds dart third_party library "quiver" version 2.1.5 to support a dependency from a Fuchsia Dart library required by the integration test. * Leverages the recent addition of TestWithEnvironment to the Fuchsia SDK, and blends this class with Googletest's (gtest) `testing::Test`, from Flutter's pre-existing gtest (third_party dependency).

WORK IN PROGRESS: Fuchsia does not yet find parent_view.cmx. I realized I need to package them, and flutter does not have build rules to build dart packages (AFAICT). The Fuchsia GN SDK (which we can't use because it lacks the Dart Fuchsia APIs) has a few other `build/` scripts that may be useful. I pulled some in, slightly tweaked them (still too many assumptions we are only building C++ packages!), and tried to rebuild. Build "appeared" successful, but re-running did not resolve the issue yet. Still working on this.

After dealing with lots of hidden issues and details, Dart packages finally appear to load.

So the test has the ability to run using a runner built from the same flutter/engine repo/state.

This also adds the fallback `fuchsia.com -> devhost` rule to `serve.sh`, which appears to be required in order to avoid running the default fuchsia package server as well.

with shell/platform/fuchsia/dart:<targets> This simplifies fuchsia SDK Dart library dependencies by assuming those targets match the versions required by Fuchsia. Since the GN target is a "fuchsia"-specific target, this is a reasonable constraint.

Replicates all five test cases, which pass.

This change requires first landing: https://flutter-review.googlesource.com/c/recipes/+/18540 which adds support for publishing more than one package per test. The CI test framework does not, currently, support running `serve.sh` as a secondary package server, so all package domains have been changed to assume `fuchsia.com`.

and trying to kill scenic.cmx before running the test

In case testing with a version of fuchsia built with the original fuchsia.git version of the tests, the flutter version will have the suffix "2", so there is no conflict or confusion.

flutter-dashboard · 2021-09-30T23:21:19Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

…from fuchsia.git (flutter#26880)

…n test) from fuchsia.git (flutter/engine#26880)

…from fuchsia.git (flutter#26880)

flutter-dashboard bot added the platform-fuchsia label Jun 22, 2021

google-cla bot added the cla: yes label Jun 22, 2021

richkadel marked this pull request as draft June 22, 2021 17:02

richkadel mentioned this pull request Jun 22, 2021

Add secondary BUILD.gn files for Dart libraries used by new tests flutter/buildroot#472

Merged

richkadel force-pushed the fuchsia-orig branch 2 times, most recently from d6f16cd to 26c5657 Compare June 22, 2021 22:41

richkadel mentioned this pull request Jun 23, 2021

reconcile and rename fuchsia variables used in GN build rules flutter/flutter#85088

Open

richkadel force-pushed the fuchsia-orig branch 2 times, most recently from fecf98e to d76802f Compare June 25, 2021 00:12

richkadel mentioned this pull request Jul 7, 2021

Adds python3_action GN target #27211

Merged

8 tasks

richkadel force-pushed the fuchsia-orig branch 2 times, most recently from 498bd52 to 7fe5d47 Compare July 7, 2021 19:47

richkadel force-pushed the fuchsia-orig branch from 7fe5d47 to 077bf0e Compare July 7, 2021 22:05

richkadel force-pushed the fuchsia-orig branch 2 times, most recently from eb479dc to 101b978 Compare September 28, 2021 19:09

arbreng approved these changes Sep 28, 2021

View reviewed changes

shell/platform/fuchsia/flutter/integration_flutter_tests/embedder/BUILD.gn Outdated Show resolved Hide resolved

arbreng requested a review from chaselatta September 28, 2021 19:12

richkadel force-pushed the fuchsia-orig branch from 101b978 to eabd4d4 Compare September 28, 2021 19:33

chaselatta approved these changes Sep 28, 2021

View reviewed changes

shell/platform/fuchsia/flutter/integration_flutter_tests/embedder/child-view/child_view.dart Outdated Show resolved Hide resolved

richkadel added 11 commits September 30, 2021 13:58

Build rules finally create runnable Dart JIT Fuchsia packages

d5463bd

After dealing with lots of hidden issues and details, Dart packages finally appear to load.

Debugging dart component launch and run issues

df8d957

Building the runner to be served alongside the test component

550a895

So the test has the ability to run using a runner built from the same flutter/engine repo/state.

Working subset of integration test, with updated README.md

59a39b0

This also adds the fallback `fuchsia.com -> devhost` rule to `serve.sh`, which appears to be required in order to avoid running the default fuchsia package server as well.

dart:ui version of original flutter test in fuchsia.git

47535f4

Replicates all five test cases, which pass.

Adding an entry for the test to test_suites.yaml

2801651

Attempting to publish jit runner to FEMU package server

5f6dbbc

and trying to kill scenic.cmx before running the test

richkadel force-pushed the fuchsia-orig branch from eabd4d4 to fe2c72c Compare September 30, 2021 22:37

Renamed integration tests files, packages, and components to add "2"

7a9face

In case testing with a version of fuchsia built with the original fuchsia.git version of the tests, the flutter version will have the suffix "2", so there is no conflict or confusion.

richkadel force-pushed the fuchsia-orig branch from fe2c72c to 7a9face Compare September 30, 2021 23:19

flutter-dashboard bot added the needs tests label Sep 30, 2021

arbreng added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Sep 30, 2021

arbreng merged commit 779740e into flutter:master Oct 1, 2021

akbiggs pushed a commit to akbiggs/engine that referenced this pull request Oct 1, 2021

Migrated integration_flutter_test/embedder (scenic integration test) …

5850c00

…from fuchsia.git (flutter#26880)

engine-flutter-autoroll mentioned this pull request Oct 1, 2021

Roll Engine from 1f8fa8041d49 to 779740ed049a (10 revisions) flutter/flutter#91070

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 1, 2021

779740e Migrated integration_flutter_test/embedder (scenic integratio…

045dbd7

…n test) from fuchsia.git (flutter/engine#26880)

engine-flutter-autoroll mentioned this pull request Oct 1, 2021

Roll Engine from 1f8fa8041d49 to 8ec71b64f6ca (12 revisions) flutter/flutter#91071

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 1, 2021

779740e Migrated integration_flutter_test/embedder (scenic integratio…

b9e9fcd

…n test) from fuchsia.git (flutter/engine#26880)

dnfield pushed a commit to dnfield/engine that referenced this pull request Oct 6, 2021

Migrated integration_flutter_test/embedder (scenic integration test) …

dcffb35

…from fuchsia.git (flutter#26880)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrated integration_flutter_test/embedder (scenic integration test) from fuchsia.git #26880

Migrated integration_flutter_test/embedder (scenic integration test) from fuchsia.git #26880

Migrated integration_flutter_test/embedder (scenic integration test) from fuchsia.git #26880

Migrated integration_flutter_test/embedder (scenic integration test) from fuchsia.git #26880

Conversation

Pre-launch Checklist