Refactor `external_ui` → `external_textures` by matanlurey · Pull Request #142062 · flutter/flutter

matanlurey · 2024-01-23T19:25:31Z

This PR makes no behavioral changes to executed code, and instead focuses on organization and naming:

Almost¹ anything named external_ui is renamed external_textures
Extended the README to explain the intent of the test, as well as how to run it
Renamed main.dart and main_test.dart to frame_rate_main.dart and frame_rate_test.dart (we'll add more)
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.

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)? ↩

…-test

matanlurey · 2024-01-24T00:48:06Z

Ping @jonahwilliams just because I've never touched these files before :)

matanlurey · 2024-01-24T00:48:38Z

/cc @jmagman as well (again, no behavioral impact - but names are changing!)

jonahwilliams

I think this will work, but I've never renamed a devicelab task before.

LGTM

keyonghan · 2024-01-24T19:41:44Z

.ci.yaml

      tags: >
        ["devicelab", "ios", "mac"]
-      task_name: external_ui_integration_test_ios
+      task_name: external_textures_integration_test_ios


This will affect the benchmark metrics as they are tied to task_name. If this is the way to go, you may want to give engine people a heads up. /cc @zanderso

These aren't benchmarks confusingly.

zanderso

lgtm

zanderso · 2024-01-24T20:41:01Z

.ci.yaml

      tags: >
        ["devicelab", "android", "linux", "pixel", "7pro"]
-      task_name: external_ui_integration_test
+      task_name: external_textures_integration_test


I believe this is only cosmetic, but I'd suggest keeping the task_name and the name field above matching. So the name above on line 2171 would change to Linux_pixel_7pro external_textures_integration_test.

Unfortunately I can't do that in a single PR because you can only rename a task by starting it as bringup: true. I can do that but I'll need a few more PRs.

eliasyishak · 2024-01-24T21:40:15Z

Reverting due to errors in post submit

Example of builder erroring out: https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_ios%20external_ui_integration_test_ios/9562/overview

Sample error

[2024-01-24 13:06:08.688564] [STDOUT] stdout: [  +59 ms] Artifact Instance of 'FlutterWebSdk' is not required, skipping update.
[2024-01-24 13:06:08.688645] [STDOUT] stdout: [        ] Artifact Instance of 'LegacyCanvasKitRemover' is not required, skipping update.
[2024-01-24 13:06:08.689204] [STDOUT] stdout: [        ] Artifact Instance of 'WindowsEngineArtifacts' is not required, skipping update.
[2024-01-24 13:06:08.689265] [STDOUT] stdout: [        ] Artifact Instance of 'MacOSEngineArtifacts' is not required, skipping update.
[2024-01-24 13:06:08.689301] [STDOUT] stdout: [        ] Artifact Instance of 'LinuxEngineArtifacts' is not required, skipping update.
[2024-01-24 13:06:08.689334] [STDOUT] stdout: [        ] Artifact Instance of 'LinuxFuchsiaSDKArtifacts' is not required, skipping update.
[2024-01-24 13:06:08.689403] [STDOUT] stdout: [        ] Artifact Instance of 'MacOSFuchsiaSDKArtifacts' is not required, skipping update.
[2024-01-24 13:06:08.689442] [STDOUT] stdout: [        ] Artifact Instance of 'FlutterRunnerSDKArtifacts' is not required, skipping update.
[2024-01-24 13:06:08.689475] [STDOUT] stdout: [        ] Artifact Instance of 'FlutterRunnerDebugSymbols' is not required, skipping update.
[2024-01-24 13:06:08.707419] [STDOUT] stdout: [  +17 ms] "flutter drive" took 20,989ms.
[2024-01-24 13:06:08.713860] [STDOUT] stderr: [   +4 ms] Target file "lib/main.dart" not found.
[2024-01-24 13:06:08.714128] [STDOUT] stderr: [   +1 ms] 
[2024-01-24 13:06:08.714185] [STDOUT] stderr:            #0      FlutterCommand.validateCommand (package:flutter_tools/src/runner/flutter_command.dart:1858:9)
[2024-01-24 13:06:08.714228] [STDOUT] stderr:            #1      DriveCommand.validateCommand (package:flutter_tools/src/commands/drive.dart:227:18)
[2024-01-24 13:06:08.714263] [STDOUT] stderr:            #2      FlutterCommand.verifyThenRunCommand (package:flutter_tools/src/runner/flutter_command.dart:1713:11)
[2024-01-24 13:06:08.714294] [STDOUT] stderr:            <asynchronous suspension>
[2024-01-24 13:06:08.714325] [STDOUT] stderr:            #3      FlutterCommand.run.<anonymous closure> (package:flutter_tools/src/runner/flutter_command.dart:1398:27)
[2024-01-24 13:06:08.714357] [STDOUT] stderr:            <asynchronous suspension>
[2024-01-24 13:06:08.714459] [STDOUT] stderr:            #4      AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:153:19)
[2024-01-24 13:06:08.714527] [STDOUT] stderr:            <asynchronous suspension>
[2024-01-24 13:06:08.714562] [STDOUT] stderr:            #5      CommandRunner.runCommand (package:args/command_runner.dart:212:13)
[2024-01-24 13:06:08.714594] [STDOUT] stderr:            <asynchronous suspension>
[2024-01-24 13:06:08.714625] [STDOUT] stderr:            #6      FlutterCommandRunner.runCommand.<anonymous closure> (package:flutter_tools/src/runner/flutter_command_runner.dart:355:9)
[2024-01-24 13:06:08.714656] [STDOUT] stderr:            <asynchronous suspension>
[2024-01-24 13:06:08.714686] [STDOUT] stderr:            #7      AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:153:19)
[2024-01-24 13:06:08.714717] [STDOUT] stderr:            <asynchronous suspension>
[2024-01-24 13:06:08.714747] [STDOUT] stderr:            #8      FlutterCommandRunner.runCommand (package:flutter_tools/src/runner/flutter_command_runner.dart:295:5)
[2024-01-24 13:06:08.714777] [STDOUT] stderr:            <asynchronous suspension>
[2024-01-24 13:06:08.714899] [STDOUT] stderr:            #9      run.<anonymous closure>.<anonymous closure> (package:flutter_tools/runner.dart:119:9)
[2024-01-24 13:06:08.714943] [STDOUT] stderr:            <asynchronous suspension>
[2024-01-24 13:06:08.714976] [STDOUT] stderr:            #10     AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:153:19)
[2024-01-24 13:06:08.715012] [STDOUT] stderr:            <asynchronous suspension>
[2024-01-24 13:06:08.715043] [STDOUT] stderr:            #11     main (package:flutter_tools/executable.dart:90:3)
[2024-01-24 13:06:08.715100] [STDOUT] stderr:            <asynchronous suspension>
[2024-01-24 13:06:08.715183] [STDOUT] stderr: 
[2024-01-24 13:06:08.715238] [STDOUT] stderr: 
[2024-01-24 13:06:08.715324] [STDOUT] stdout: [        ] ensureAnalyticsSent: 0ms
[2024-01-24 13:06:08.715362] [STDOUT] stdout: [        ] Running 1 shutdown hook
[2024-01-24 13:06:08.715392] [STDOUT] stdout: [        ] Shutdown hooks complete
[2024-01-24 13:06:08.715422] [STDOUT] stdout: [        ] exiting with code 1

This reverts commit 2e2042f.

Reverts #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)?

yusuf-goog · 2024-01-24T21:55:15Z

@eliasyishak do you need to add the revert label to this issue?

eliasyishak · 2024-01-24T21:56:54Z

Yep it was my mistake to open that other PR manually @yusuf-goog, the bot already created a new PR and reverted this change

…#142062)

Roll Flutter from 19b06f4 to a8efa77 (38 revisions) flutter/flutter@19b06f4...a8efa77 2024-01-25 [email protected] Roll Flutter Engine from e2014f007f61 to 7c4ed15cb271 (1 revision) (flutter/flutter#142221) 2024-01-25 [email protected] Roll Flutter Engine from 5fa2e2920274 to e2014f007f61 (1 revision) (flutter/flutter#142213) 2024-01-25 [email protected] provide command to `FakeCommand::onRun` (flutter/flutter#142206) 2024-01-25 [email protected] Roll Flutter Engine from c346fd3d9ca1 to 5fa2e2920274 (1 revision) (flutter/flutter#142212) 2024-01-25 [email protected] Roll Flutter Engine from d4d8f668159b to c346fd3d9ca1 (1 revision) (flutter/flutter#142209) 2024-01-25 [email protected] Roll Flutter Engine from 499ed00bbda2 to d4d8f668159b (2 revisions) (flutter/flutter#142205) 2024-01-25 [email protected] Roll Flutter Engine from 4880592ca5ba to 499ed00bbda2 (6 revisions) (flutter/flutter#142202) 2024-01-25 [email protected] [ci] Adds test for web hot restart with const App. (flutter/flutter#141824) 2024-01-25 [email protected] Migrate android_view to linux_android_emu platform. (flutter/flutter#142184) 2024-01-25 [email protected] Refactor `external_ui` without making any name changes (I think) (flutter/flutter#142192) 2024-01-25 [email protected] Fix text selection edge scrolling when inside a horizontal scrollable (flutter/flutter#140250) 2024-01-25 [email protected] Roll Flutter Engine from d7bf5ec1dcdd to 4880592ca5ba (2 revisions) (flutter/flutter#142186) 2024-01-24 [email protected] Roll Flutter Engine from 6a7f963dc751 to d7bf5ec1dcdd (2 revisions) (flutter/flutter#142185) 2024-01-24 [email protected] Reland "Remove hack from PageView." (flutter/flutter#142172) 2024-01-24 [email protected] Upgrade leak_tracker. (flutter/flutter#142162) 2024-01-24 [email protected] Migrate android views to devicelab. (flutter/flutter#142081) 2024-01-24 [email protected] Roll Flutter Engine from ed498f111d53 to 6a7f963dc751 (4 revisions) (flutter/flutter#142176) 2024-01-24 [email protected] Support wireless debugging for iOS 12 or earlier (flutter/flutter#141439) 2024-01-24 [email protected] Roll Flutter Engine from 4bc368ee5f74 to ed498f111d53 (5 revisions) (flutter/flutter#142167) 2024-01-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Refactor `external_ui` â�� `external_textures`" (flutter/flutter#142173) 2024-01-24 [email protected] Refactor `external_ui` â�� `external_textures` (flutter/flutter#142062) 2024-01-24 [email protected] Roll Flutter Engine from 0b9fce355df9 to 4bc368ee5f74 (3 revisions) (flutter/flutter#142157) 2024-01-24 [email protected] Update navigationBar label's maxScaleFactor to meet GAR requirement (flutter/flutter#141998) 2024-01-24 [email protected] Roll Flutter Engine from b65556fa543e to 0b9fce355df9 (1 revision) (flutter/flutter#142147) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_2_4 to be unflaky (flutter/flutter#142115) 2024-01-24 [email protected] Marks Mac_x64 build_tests_2_4 to be unflaky (flutter/flutter#142111) 2024-01-24 [email protected] Marks Mac_x64 build_tests_1_4 to be unflaky (flutter/flutter#142110) 2024-01-24 [email protected] Marks Mac_x64 build_tests_3_4 to be unflaky (flutter/flutter#142112) 2024-01-24 [email protected] Revise tooltip theme docs, including more cross-references (flutter/flutter#137316) 2024-01-24 [email protected] Run some tests explicitly in both arm and x64. (flutter/flutter#141910) 2024-01-24 [email protected] Roll Flutter Engine from ba3a70ce7722 to b65556fa543e (3 revisions) (flutter/flutter#142140) 2024-01-24 [email protected] Fixes #138773, port autocomplete to OverlayPortal (flutter/flutter#140285) 2024-01-24 [email protected] Revert "[web] - Fix broken `TextField` in semantics mode when it's a sibling of `Navigator`" (flutter/flutter#142129) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_1_4 to be unflaky (flutter/flutter#142114) 2024-01-24 [email protected] Roll Packages from 841fe90 to 8fbdf65 (2 revisions) (flutter/flutter#142139) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_3_4 to be unflaky (flutter/flutter#142116) 2024-01-24 [email protected] Marks Mac_x64 build_tests_4_4 to be unflaky (flutter/flutter#142113) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_4_4 to be unflaky (flutter/flutter#142117) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose ...

matanlurey added 3 commits January 23, 2024 11:24

Rename and refactor external_textures.

9f4d0fb

++

e227fe0

++

d61a6e4

github-actions bot added the a: text input Entering text in a text field or keyboard related problems label Jan 23, 2024

matanlurey changed the title ~~WIP: Rename and refactor external_textures.~~ Refactor external_ui → external_textures Jan 23, 2024

matanlurey requested review from johnmccutchan and jonahwilliams January 23, 2024 21:06

matanlurey marked this pull request as ready for review January 23, 2024 21:07

johnmccutchan approved these changes Jan 23, 2024

View reviewed changes

matanlurey added 2 commits January 23, 2024 14:34

++

076047d

Merge remote-tracking branch 'upstream/master' into external-textures…

c1c5699

…-test

matanlurey force-pushed the external-textures-test branch from d80f164 to c1c5699 Compare January 23, 2024 22:34

++

63bf7d7

jonahwilliams approved these changes Jan 24, 2024

View reviewed changes

matanlurey requested a review from yusuf-goog January 24, 2024 01:24

keyonghan reviewed Jan 24, 2024

View reviewed changes

zanderso approved these changes Jan 24, 2024

View reviewed changes

matanlurey merged commit 2e2042f into flutter:master Jan 24, 2024

eliasyishak mentioned this pull request Jan 24, 2024

Revert "Refactor external_ui → external_textures" #142171

Closed

eliasyishak added the revert Autorevert PR (with "Reason for revert:" comment) label Jan 24, 2024

auto-submit bot pushed a commit that referenced this pull request Jan 24, 2024

Revert "Refactor external_ui → external_textures (#142062)"

56f8a8a

This reverts commit 2e2042f.

auto-submit bot mentioned this pull request Jan 24, 2024

Reverts "Refactor external_ui → external_textures" #142173

Merged

auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Jan 24, 2024

matanlurey mentioned this pull request Jan 24, 2024

Re-land: Refactor external_ui → external_textures #142178

Closed

3 tasks

engine-flutter-autoroll mentioned this pull request Jan 25, 2024

Roll Flutter from 19b06f4ee9a0 to a8efa771d6a3 (38 revisions) flutter/packages#5969

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 25, 2024

2e2042f Refactor external_ui → external_textures (flutter/flutter…

59fda46

…#142062)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor `external_ui` → `external_textures`#142062

Refactor `external_ui` → `external_textures`#142062
matanlurey merged 6 commits intoflutter:masterfrom
matanlurey:external-textures-test

matanlurey commented Jan 23, 2024 •

edited

Loading

Uh oh!

matanlurey commented Jan 24, 2024

Uh oh!

matanlurey commented Jan 24, 2024

Uh oh!

jonahwilliams left a comment

Uh oh!

keyonghan Jan 24, 2024

Uh oh!

matanlurey Jan 24, 2024

Uh oh!

zanderso left a comment

Uh oh!

zanderso Jan 24, 2024

Uh oh!

matanlurey Jan 24, 2024

Uh oh!

eliasyishak commented Jan 24, 2024

Uh oh!

yusuf-goog commented Jan 24, 2024

Uh oh!

eliasyishak commented Jan 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

matanlurey commented Jan 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

matanlurey commented Jan 24, 2024

Uh oh!

matanlurey commented Jan 24, 2024

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

keyonghan Jan 24, 2024

Choose a reason for hiding this comment

Uh oh!

matanlurey Jan 24, 2024

Choose a reason for hiding this comment

Uh oh!

zanderso left a comment

Choose a reason for hiding this comment

Uh oh!

zanderso Jan 24, 2024

Choose a reason for hiding this comment

Uh oh!

matanlurey Jan 24, 2024

Choose a reason for hiding this comment

Uh oh!

eliasyishak commented Jan 24, 2024

Uh oh!

yusuf-goog commented Jan 24, 2024

Uh oh!

eliasyishak commented Jan 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

matanlurey commented Jan 23, 2024 •

edited

Loading