From 54e9f2dbe6ca6a6ead0019a1c19c1af1e4e435ba Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Wed, 24 Jan 2024 21:41:17 +0000 Subject: [PATCH] =?UTF-8?q?Reverts=20"Refactor=20`external=5Fui`=20?= =?UTF-8?q?=E2=86=92=20`external=5Ftextures`"=20(#142173)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts flutter/flutter#142062 Initiated by: eliasyishak This change reverts the following previous change: Original Description: This PR makes no _behavioral_ changes to executed code, and instead focuses on organization and naming: 1. Almost[^1] anything named `external_ui` is renamed `external_textures` 1. Extended the README to explain the intent of the test, as well as how to run it 1. Renamed `main.dart` and `main_test.dart` to `frame_rate_main.dart` and `frame_rate_test.dart` (we'll add more) 1. Did some refactoring of the test to make it more obvious what is being asserted (i.e. `widgetBuilds` and friends) Given how complex (and in-flux) this directory is, I'm also requesting either John, Jonah or I review any changes. [^1]: Except the name of the `.ci.yaml` task, i.e. `name: Linux_pixel_7pro external_ui_integration_test` because I'm apparently not able to change that without creating a new task as `bringup: true` and playing a bit of a dance. Maybe that's worth doing though (in future PRs)? --- .ci.yaml | 4 +- CODEOWNERS | 1 - TESTOWNERS | 4 +- ...dart => external_ui_integration_test.dart} | 2 +- ... => external_ui_integration_test_ios.dart} | 2 +- .../lib/tasks/integration_tests.dart | 4 +- .../external_textures/README.md | 47 ------------ .../test_driver/frame_rate_main_test.dart | 73 ------------------- dev/integration_tests/external_ui/README.md | 3 + .../android/app/build.gradle | 0 .../android/app/src/main/AndroidManifest.xml | 2 +- .../io/flutter/externalui/MainActivity.java | 0 .../android/build.gradle | 0 .../android/buildscript-gradle.lockfile | 0 .../android/gradle.properties | 0 .../gradle/wrapper/gradle-wrapper.properties | 0 .../android/project-app.lockfile | 0 .../android/settings.gradle | 0 .../ios/Flutter/AppFrameworkInfo.plist | 0 .../ios/Flutter/Debug.xcconfig | 0 .../ios/Flutter/Release.xcconfig | 0 .../ios/Runner.xcodeproj/project.pbxproj | 0 .../contents.xcworkspacedata | 0 .../xcshareddata/IDEWorkspaceChecks.plist | 0 .../xcshareddata/xcschemes/Runner.xcscheme | 0 .../contents.xcworkspacedata | 0 .../ios/Runner/AppDelegate.h | 0 .../ios/Runner/AppDelegate.m | 0 .../Runner/Base.lproj/LaunchScreen.storyboard | 0 .../ios/Runner/Base.lproj/Main.storyboard | 0 .../ios/Runner/Info.plist | 2 +- .../ios/Runner/main.m | 0 .../lib/main.dart} | 0 .../pubspec.yaml | 2 +- .../external_ui/test_driver/main_test.dart | 68 +++++++++++++++++ 35 files changed, 82 insertions(+), 132 deletions(-) rename dev/devicelab/bin/tasks/{external_textures_integration_test.dart => external_ui_integration_test.dart} (88%) rename dev/devicelab/bin/tasks/{external_textures_integration_test_ios.dart => external_ui_integration_test_ios.dart} (88%) delete mode 100644 dev/integration_tests/external_textures/README.md delete mode 100644 dev/integration_tests/external_textures/test_driver/frame_rate_main_test.dart create mode 100644 dev/integration_tests/external_ui/README.md rename dev/integration_tests/{external_textures => external_ui}/android/app/build.gradle (100%) rename dev/integration_tests/{external_textures => external_ui}/android/app/src/main/AndroidManifest.xml (97%) rename dev/integration_tests/{external_textures => external_ui}/android/app/src/main/java/io/flutter/externalui/MainActivity.java (100%) rename dev/integration_tests/{external_textures => external_ui}/android/build.gradle (100%) rename dev/integration_tests/{external_textures => external_ui}/android/buildscript-gradle.lockfile (100%) rename dev/integration_tests/{external_textures => external_ui}/android/gradle.properties (100%) rename dev/integration_tests/{external_textures => external_ui}/android/gradle/wrapper/gradle-wrapper.properties (100%) rename dev/integration_tests/{external_textures => external_ui}/android/project-app.lockfile (100%) rename dev/integration_tests/{external_textures => external_ui}/android/settings.gradle (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Flutter/AppFrameworkInfo.plist (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Flutter/Debug.xcconfig (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Flutter/Release.xcconfig (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner.xcodeproj/project.pbxproj (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner.xcodeproj/project.xcworkspace/contents.xcworkspacedata (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner.xcworkspace/contents.xcworkspacedata (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner/AppDelegate.h (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner/AppDelegate.m (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner/Base.lproj/LaunchScreen.storyboard (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner/Base.lproj/Main.storyboard (100%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner/Info.plist (97%) rename dev/integration_tests/{external_textures => external_ui}/ios/Runner/main.m (100%) rename dev/integration_tests/{external_textures/lib/frame_rate_main.dart => external_ui/lib/main.dart} (100%) rename dev/integration_tests/{external_textures => external_ui}/pubspec.yaml (99%) create mode 100644 dev/integration_tests/external_ui/test_driver/main_test.dart diff --git a/.ci.yaml b/.ci.yaml index c6f5b372467..d797d81a86f 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -2175,7 +2175,7 @@ targets: properties: tags: > ["devicelab", "android", "linux", "pixel", "7pro"] - task_name: external_textures_integration_test + task_name: external_ui_integration_test # linux motog4 benchmark - name: Linux_android fading_child_animation_perf__timeline_summary @@ -4281,7 +4281,7 @@ targets: properties: tags: > ["devicelab", "ios", "mac"] - task_name: external_textures_integration_test_ios + task_name: external_ui_integration_test_ios ignore_flakiness: "true" - name: Mac_ios route_test_ios diff --git a/CODEOWNERS b/CODEOWNERS index 3602867f835..bb11af9e98e 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -9,4 +9,3 @@ /packages/flutter_tools/pubspec.yaml @christopherfujino /packages/flutter_tools/templates/module/ios/ @jmagman /packages/flutter_tools/templates/**/Podfile* @jmagman -/dev/integration_tests/external_textures/** @matanlurey @johnmccutchan @jonahwilliams diff --git a/TESTOWNERS b/TESTOWNERS index 8f918a4a3b5..af68657116d 100644 --- a/TESTOWNERS +++ b/TESTOWNERS @@ -129,7 +129,7 @@ /dev/devicelab/bin/tasks/cull_opacity_perf__timeline_summary.dart @zanderso @flutter/engine /dev/devicelab/bin/tasks/drive_perf_debug_warning.dart @zanderso @flutter/engine /dev/devicelab/bin/tasks/embedded_android_views_integration_test.dart @stuartmorgan @flutter/plugin -/dev/devicelab/bin/tasks/external_textures_integration_test.dart @zanderso @flutter/engine +/dev/devicelab/bin/tasks/external_ui_integration_test.dart @zanderso @flutter/engine /dev/devicelab/bin/tasks/fading_child_animation_perf__timeline_summary.dart @zanderso @flutter/engine /dev/devicelab/bin/tasks/fast_scroll_large_images__memory.dart @zanderso @flutter/engine /dev/devicelab/bin/tasks/flavors_test.dart @zanderso @flutter/tool @@ -174,7 +174,7 @@ /dev/devicelab/bin/tasks/complex_layout_scroll_perf_bad_ios__timeline_summary.dart @jonahwilliams @flutter/engine /dev/devicelab/bin/tasks/complex_layout_scroll_perf_ios__timeline_summary.dart @vashworth @flutter/engine /dev/devicelab/bin/tasks/cubic_bezier_perf_ios_sksl_warmup__timeline_summary.dart @zanderso @flutter/engine -/dev/devicelab/bin/tasks/external_textures_integration_test_ios.dart @zanderso @flutter/engine +/dev/devicelab/bin/tasks/external_ui_integration_test_ios.dart @zanderso @flutter/engine /dev/devicelab/bin/tasks/flavors_test_ios.dart @vashworth @flutter/tool /dev/devicelab/bin/tasks/flavors_test_ios_xcode_debug.dart @vashworth @flutter/tool /dev/devicelab/bin/tasks/flutter_gallery__transition_perf_e2e_ios.dart @zanderso @flutter/engine diff --git a/dev/devicelab/bin/tasks/external_textures_integration_test.dart b/dev/devicelab/bin/tasks/external_ui_integration_test.dart similarity index 88% rename from dev/devicelab/bin/tasks/external_textures_integration_test.dart rename to dev/devicelab/bin/tasks/external_ui_integration_test.dart index 1e61b1cc4a3..cc0930ea734 100644 --- a/dev/devicelab/bin/tasks/external_textures_integration_test.dart +++ b/dev/devicelab/bin/tasks/external_ui_integration_test.dart @@ -8,5 +8,5 @@ import 'package:flutter_devicelab/tasks/integration_tests.dart'; Future main() async { deviceOperatingSystem = DeviceOperatingSystem.android; - await task(createExternalTexturesIntegrationTest()); + await task(createExternalUiIntegrationTest()); } diff --git a/dev/devicelab/bin/tasks/external_textures_integration_test_ios.dart b/dev/devicelab/bin/tasks/external_ui_integration_test_ios.dart similarity index 88% rename from dev/devicelab/bin/tasks/external_textures_integration_test_ios.dart rename to dev/devicelab/bin/tasks/external_ui_integration_test_ios.dart index e0d2990e991..a573cc1b611 100644 --- a/dev/devicelab/bin/tasks/external_textures_integration_test_ios.dart +++ b/dev/devicelab/bin/tasks/external_ui_integration_test_ios.dart @@ -8,5 +8,5 @@ import 'package:flutter_devicelab/tasks/integration_tests.dart'; Future main() async { deviceOperatingSystem = DeviceOperatingSystem.ios; - await task(createExternalTexturesIntegrationTest()); + await task(createExternalUiIntegrationTest()); } diff --git a/dev/devicelab/lib/tasks/integration_tests.dart b/dev/devicelab/lib/tasks/integration_tests.dart index 2ce063973b1..f3177e1f10f 100644 --- a/dev/devicelab/lib/tasks/integration_tests.dart +++ b/dev/devicelab/lib/tasks/integration_tests.dart @@ -40,9 +40,9 @@ TaskFunction createIntegrationTestFlavorsTest({Map? environment} ).call; } -TaskFunction createExternalTexturesIntegrationTest() { +TaskFunction createExternalUiIntegrationTest() { return DriverTest( - '${flutterDirectory.path}/dev/integration_tests/external_textures', + '${flutterDirectory.path}/dev/integration_tests/external_ui', 'lib/main.dart', ).call; } diff --git a/dev/integration_tests/external_textures/README.md b/dev/integration_tests/external_textures/README.md deleted file mode 100644 index 46627acbfe2..00000000000 --- a/dev/integration_tests/external_textures/README.md +++ /dev/null @@ -1,47 +0,0 @@ -# external_textures - -Tests external texture rendering between a native[^1] platform and Flutter. - -Part of Flutter's API for [plugins](https://flutter.dev/docs/development/packages-and-plugins/developing-packages#plugin) includes passing _external textures_, or textures -created outside of Flutter, to Flutter, typically using the [`Texture`][texture] -widget. This is useful for plugins that render video, or for plugins that -interact with the camera. - -For example: - -- [`packages/camera`][camera] -- [`packages/video_player`][video_player] - -[texture]: https://api.flutter.dev/flutter/widgets/Texture-class.html -[camera]: https://github.com/flutter/packages/tree/8255fbed74465425a1ec06a1804225e705e29f52/packages/camera -[video_player]: https://github.com/flutter/packages/tree/8255fbed74465425a1ec06a1804225e705e29f52/packages/video_player - -Because external textures are created outside of Flutter, there is often subtle -translation that needs to happen between the native platform and Flutter, which -is hard to observe. These integration tests are designed to help catch these -subtle translation issues. - -## How it works - -- Each `lib/*_main.dart` file is a Flutter app instrumenting a test case. -- There is a cooresponding `test_driver/*_test.dart` that runs assertions. - -To run the test cases locally, use `flutter drive`[^2]: - -```shell -flutter drive lib/frame_rate_main.dart --driver test_driver/frame_rate_test.dart -``` - -> [!TIP] -> On CI, the test cases are run within our [device lab](../../devicelab/README.md). -> -> See [`devicelab/lib/tasks/integration_tests.dart`](../../devicelab/lib/tasks/integration_tests.dart) -> and search for `createExternalTexturesIntegrationTest`. -> -> The actual tests are run by task runners: -> -> - [Android](../../devicelab/bin/tasks/external_textures_integration_test.dart) -> - [iOS](../../devicelab/bin/tasks/external_textures_integration_test_ios.dart) - -[^1]: Only iOS and Android. -[^2]: Unfortunately documentation is quite limited. See [#142021](https://github.com/flutter/flutter/issues/142021). diff --git a/dev/integration_tests/external_textures/test_driver/frame_rate_main_test.dart b/dev/integration_tests/external_textures/test_driver/frame_rate_main_test.dart deleted file mode 100644 index fef633fa4ba..00000000000 --- a/dev/integration_tests/external_textures/test_driver/frame_rate_main_test.dart +++ /dev/null @@ -1,73 +0,0 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'dart:async'; - -import 'package:flutter_driver/flutter_driver.dart'; -import 'package:test/test.dart' hide TypeMatcher, isInstanceOf; - -final RegExp _calibrationRegExp = RegExp('Flutter frame rate is (.*)fps'); -final RegExp _statsRegExp = RegExp('Produced: (.*)fps\nConsumed: (.*)fps\nWidget builds: (.*)'); -const Duration _samplingTime = Duration(seconds: 8); - -Future main() async { - late final FlutterDriver driver; - - setUpAll(() async { - driver = await FlutterDriver.connect(); - }); - - tearDownAll(() async { - await driver.close(); - }); - - // Verifies we consume texture frames at a rate close to the minimum of the - // rate at which they are produced and Flutter's frame rate. In addition, - // it verifies that widget builds are not triggered by external texture - // frames. - test('renders frames from the device at a rate similar to the frames produced', () async { - final SerializableFinder fab = find.byValueKey('fab'); - final SerializableFinder summary = find.byValueKey('summary'); - - // Wait for calibration to complete and fab to appear. - await driver.waitFor(fab); - - final String calibrationResult = await driver.getText(summary); - final Match? matchCalibration = _calibrationRegExp.matchAsPrefix(calibrationResult); - expect(matchCalibration, isNotNull); - final double flutterFrameRate = double.parse(matchCalibration?.group(1) ?? '0'); - - // Texture frame stats at 0.5x Flutter frame rate - await driver.tap(fab); - await Future.delayed(_samplingTime); - await driver.tap(fab); - - final String statsSlow = await driver.getText(summary); - final Match matchSlow = _statsRegExp.matchAsPrefix(statsSlow)!; - expect(matchSlow, isNotNull); - - double framesProduced = double.parse(matchSlow.group(1)!); - expect(framesProduced, closeTo(flutterFrameRate / 2.0, 5.0)); - double framesConsumed = double.parse(matchSlow.group(2)!); - expect(framesConsumed, closeTo(flutterFrameRate / 2.0, 5.0)); - int widgetBuilds = int.parse(matchSlow.group(3)!); - expect(widgetBuilds, 1); - - // Texture frame stats at 2.0x Flutter frame rate - await driver.tap(fab); - await Future.delayed(_samplingTime); - await driver.tap(fab); - - final String statsFast = await driver.getText(summary); - final Match matchFast = _statsRegExp.matchAsPrefix(statsFast)!; - expect(matchFast, isNotNull); - - framesProduced = double.parse(matchFast.group(1)!); - expect(framesProduced, closeTo(flutterFrameRate * 2.0, 5.0)); - framesConsumed = double.parse(matchFast.group(2)!); - expect(framesConsumed, closeTo(flutterFrameRate, 10.0)); - widgetBuilds = int.parse(matchSlow.group(3)!); - expect(widgetBuilds, 1); - }, timeout: Timeout.none); -} diff --git a/dev/integration_tests/external_ui/README.md b/dev/integration_tests/external_ui/README.md new file mode 100644 index 00000000000..a51e9462ffe --- /dev/null +++ b/dev/integration_tests/external_ui/README.md @@ -0,0 +1,3 @@ +# external_ui + +A Flutter project for testing external texture rendering. diff --git a/dev/integration_tests/external_textures/android/app/build.gradle b/dev/integration_tests/external_ui/android/app/build.gradle similarity index 100% rename from dev/integration_tests/external_textures/android/app/build.gradle rename to dev/integration_tests/external_ui/android/app/build.gradle diff --git a/dev/integration_tests/external_textures/android/app/src/main/AndroidManifest.xml b/dev/integration_tests/external_ui/android/app/src/main/AndroidManifest.xml similarity index 97% rename from dev/integration_tests/external_textures/android/app/src/main/AndroidManifest.xml rename to dev/integration_tests/external_ui/android/app/src/main/AndroidManifest.xml index b8198efc1c9..505106a5c3b 100644 --- a/dev/integration_tests/external_textures/android/app/src/main/AndroidManifest.xml +++ b/dev/integration_tests/external_ui/android/app/src/main/AndroidManifest.xml @@ -9,7 +9,7 @@ found in the LICENSE file. --> + android:label="external_ui"> CFBundleInfoDictionaryVersion 6.0 CFBundleName - external_textures + external_ui CFBundlePackageType APPL CFBundleShortVersionString diff --git a/dev/integration_tests/external_textures/ios/Runner/main.m b/dev/integration_tests/external_ui/ios/Runner/main.m similarity index 100% rename from dev/integration_tests/external_textures/ios/Runner/main.m rename to dev/integration_tests/external_ui/ios/Runner/main.m diff --git a/dev/integration_tests/external_textures/lib/frame_rate_main.dart b/dev/integration_tests/external_ui/lib/main.dart similarity index 100% rename from dev/integration_tests/external_textures/lib/frame_rate_main.dart rename to dev/integration_tests/external_ui/lib/main.dart diff --git a/dev/integration_tests/external_textures/pubspec.yaml b/dev/integration_tests/external_ui/pubspec.yaml similarity index 99% rename from dev/integration_tests/external_textures/pubspec.yaml rename to dev/integration_tests/external_ui/pubspec.yaml index da36b4d9c36..099d782fc57 100644 --- a/dev/integration_tests/external_textures/pubspec.yaml +++ b/dev/integration_tests/external_ui/pubspec.yaml @@ -1,4 +1,4 @@ -name: external_textures +name: external_ui description: A test of Flutter integrating external UIs. environment: diff --git a/dev/integration_tests/external_ui/test_driver/main_test.dart b/dev/integration_tests/external_ui/test_driver/main_test.dart new file mode 100644 index 00000000000..474663e0237 --- /dev/null +++ b/dev/integration_tests/external_ui/test_driver/main_test.dart @@ -0,0 +1,68 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; + +import 'package:flutter_driver/flutter_driver.dart'; +import 'package:test/test.dart' hide TypeMatcher, isInstanceOf; + +final RegExp calibrationRegExp = RegExp('Flutter frame rate is (.*)fps'); +final RegExp statsRegExp = RegExp('Produced: (.*)fps\nConsumed: (.*)fps\nWidget builds: (.*)'); +const Duration samplingTime = Duration(seconds: 8); + +Future main() async { + group('texture suite', () { + late FlutterDriver driver; + + setUpAll(() async { + driver = await FlutterDriver.connect(); + }); + + // This test verifies that we can consume texture frames at a rate + // close to the minimum of the rate at which they are produced + // and Flutter's frame rate. It also verifies that we do not rebuild the + // Widget tree during texture consumption. The test starts by measuring + // Flutter's frame rate. + test('texture rendering', () async { + final SerializableFinder fab = find.byValueKey('fab'); + final SerializableFinder summary = find.byValueKey('summary'); + + // Wait for calibration to complete and fab to appear. + await driver.waitFor(fab); + + final String calibrationResult = await driver.getText(summary); + final Match? matchCalibration = calibrationRegExp.matchAsPrefix(calibrationResult); + expect(matchCalibration, isNotNull); + final double flutterFrameRate = double.parse(matchCalibration?.group(1) ?? '0'); + + // Texture frame stats at 0.5x Flutter frame rate + await driver.tap(fab); + await Future.delayed(samplingTime); + await driver.tap(fab); + + final String statsSlow = await driver.getText(summary); + final Match matchSlow = statsRegExp.matchAsPrefix(statsSlow)!; + expect(matchSlow, isNotNull); + expect(double.parse(matchSlow.group(1)!), closeTo(flutterFrameRate / 2.0, 5.0)); + expect(double.parse(matchSlow.group(2)!), closeTo(flutterFrameRate / 2.0, 5.0)); + expect(int.parse(matchSlow.group(3)!), 1); + + // Texture frame stats at 2.0x Flutter frame rate + await driver.tap(fab); + await Future.delayed(samplingTime); + await driver.tap(fab); + + final String statsFast = await driver.getText(summary); + final Match matchFast = statsRegExp.matchAsPrefix(statsFast)!; + expect(matchFast, isNotNull); + expect(double.parse(matchFast.group(1)!), closeTo(flutterFrameRate * 2.0, 5.0)); + expect(double.parse(matchFast.group(2)!), closeTo(flutterFrameRate, 10.0)); + expect(int.parse(matchFast.group(3)!), 1); + }, timeout: Timeout.none); + + tearDownAll(() async { + driver.close(); + }); + }); +}