Re-land [macOS] Bring up "flutter_gallery" devicelab, start up test for x86. by a-wallen · Pull Request #110379 · flutter/flutter

a-wallen · 2022-08-26T22:01:51Z

Reason

The initial desktop integration tests are running now, but they need to be expanded beyond the initial gallery build test; filing this to make sure that planned work is captured in the issue tracker. This should be much more straightforward that getting the first one running was, since the bot work is all complete.

In particular we should ensure we are testing building a freshly created project to catch errors in template changes. (I haven't checked where that test lives on existing platforms; it may be a device lab test, which still needs desktop bring-up).

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], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
All existing and new tests are passing.

christopherfujino · 2022-08-26T22:46:24Z

dev/devicelab/bin/tasks/flutter_gallery_macos__start_up.dart

rather than renaming the old file, don't you want to copy it to create a new one?

I removed the file in fd94568937c57b364172a4bc57e223260be1b8eb, created a new one in 3764fdbe3653b1b71c58823f9ed87b91a290efa8, and copied it over in 27be22991ab7e252d2b4397f7f32856a0011ed0c.

I could be misunderstanding the suggestion. Is the suggestion that I keep both tests/tasks?

Don't we want to test starting on android and macOS?

Yes, you're totally right and I missed that. Thanks!

Good point @zanderso. I tried to audit all the Mac/Android benchmarks in #74522 but that one was special per #73907 (#94980) because this was originally used to test the tool on different macOS versions? #38567

Anyway you're right, we should just delete flutter_gallery_mac__start_up.

Mac_android microbenchmarks

We kept this on Mac because it was flaky on Linux, though maybe that isn't true anymore: #74522 (comment) #95366

Mac_android hot_mode_dev_cycle__benchmark

Mac_android hello_world_android__compile

These should stay on Mac_android as they are testing performance/behavior between the Mac and Android devices. Per #74522

Host-dependent tests (should not be moved)

run_release_test

hello_world_android__compile

hot_mode_dev_cycle__benchmark

This PR is updated with the changes discussed in the thread. The diff does not accurately reflect my intentions so to be clear:

I created the new task (flutter_gallery_macos__start_up) in 6be515f6e1d91e65a6b5aa149da7ae904d3a2e78 and deleted flutter_gallery_mac__start_up in ce8ee07258a646ecec099719584ed74c5e115299

The diff, on the other hand, shows that I simply renamed the file.

@christopherfujino It's probably enough to just benchmark Linux/Android.

It doesn't look like we have a benchmark that records time to first frame on android.

dev/devicelab/bin/tasks/

flutter_gallery_android__compile /* MISSING: flutter_gallery_android__start_up */ flutter_gallery_ios__compile flutter_gallery_ios__start_up flutter_gallery_macos__compile flutter_gallery_macos__start_up flutter_gallery_win_desktop__compile flutter_gallery_win_desktop__start_up

@christopherfujino It's probably enough to just benchmark Linux/Android.

It doesn't look like we have a benchmark that records time to first frame on android.

dev/devicelab/bin/tasks/

flutter_gallery_android__compile /* MISSING: flutter_gallery_android__start_up */ flutter_gallery_ios__compile flutter_gallery_ios__start_up flutter_gallery_macos__compile flutter_gallery_macos__start_up flutter_gallery_win_desktop__compile flutter_gallery_win_desktop__start_up

unforunately android is usually implicit in these old devicelab tests

https://github.com/flutter/flutter/blob/master/dev/devicelab/bin/tasks/flutter_gallery__start_up.dart

jmagman

In this PR I suggest:

Rename flutter_gallery_mac__start_up.dart to flutter_gallery_android__start_up.dart. Only update task_name: flutter_gallery_android__start_up but leave name: Mac_android flutter_gallery_mac__start_up.
Update TESTOWNERS to /dev/devicelab/bin/tasks/flutter_gallery_android__start_up.dart @zanderso @flutter/engine
Create a new file flutter_gallery_macos__start_up.dart with deviceOperatingSystem = DeviceOperatingSystem.macos;.

That will add your macOS benchmark, and prevent breakage of the Android benchmark and keep the data continuous.

jmagman · 2022-08-29T17:30:03Z

dev/devicelab/bin/tasks/flutter_gallery_macos__start_up.dart

You can change the file name and task_name but leave the name as flutter_gallery_mac__start_up, that will prevent the benchmark breakage and will leave a nugget that that data is for Android.

christopherfujino · 2022-08-29T17:51:17Z

In this PR I suggest:

Rename flutter_gallery_mac__start_up.dart to flutter_gallery_android__start_up.dart. Only update task_name: flutter_gallery_android__start_up but leave name: Mac_android flutter_gallery_mac__start_up.

Update TESTOWNERS to /dev/devicelab/bin/tasks/flutter_gallery_android__start_up.dart @zanderso @flutter/engine

Create a new file flutter_gallery_macos__start_up.dart with deviceOperatingSystem = DeviceOperatingSystem.macos;.

That will add your macOS benchmark, and prevent breakage of the Android benchmark and keep the data continuous.

Is it not task_name that is used as the key in the datastore?

TESTOWNERS

… macos

a-wallen · 2022-09-08T00:08:44Z

👋 @jmagman thanks for working with me here. Now that both

Remove flutter_gallery_mac__start_up as target in .ci.yaml #111117
Delete flutter_gallery_mac__start_up. #111147
have landed, this PR is as atomic as it will get. All feedback to this point is addressed; therefore, I am re-requesting your approval.

jmagman

LGTM, thanks for all the intermediary work. I promise most Flutter work isn't this tedious 🙂

…p test for x86. (flutter/flutter#110379)

flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Aug 26, 2022

a-wallen force-pushed the macos_bringup_gallery_startup branch 3 times, most recently from b446d02 to c34bf56 Compare August 26, 2022 22:44

a-wallen requested review from christopherfujino and jmagman August 26, 2022 22:45

christopherfujino reviewed Aug 26, 2022

View reviewed changes

a-wallen marked this pull request as ready for review August 26, 2022 23:40

a-wallen requested a review from keyonghan as a code owner August 26, 2022 23:40

jmagman requested changes Aug 29, 2022

View reviewed changes

a-wallen force-pushed the macos_bringup_gallery_startup branch 3 times, most recently from cbf7c72 to 091c30e Compare August 29, 2022 20:59

a-wallen force-pushed the macos_bringup_gallery_startup branch from 091c30e to 64887a8 Compare August 30, 2022 20:13

christopherfujino reviewed Aug 30, 2022

View reviewed changes

TESTOWNERS Outdated Show resolved Hide resolved

a-wallen requested a review from jmagman September 1, 2022 22:46

a-wallen force-pushed the macos_bringup_gallery_startup branch from 718c5b3 to 57b04c7 Compare September 5, 2022 22:27

This was referenced Sep 7, 2022

Remove flutter_gallery_mac__start_up. #111143

Closed

Delete flutter_gallery_mac__start_up. #111147

Merged

Alex Wallen added 5 commits September 7, 2022 17:04

Create flutter_gallery_macos_start_up.dart

a6dc803

Create process entrypoint for createFlutterGalleryStartUpTest task on…

019ab09

… macos

Register new target to run flutter_gallery_macos_task on mac platforms

a4db5a9

Claim ownership of flutter_gallery_macos__start_up.dart in TESTOWNERS

d96aad7

Remove presubmit for benchmark test

64d46cd

a-wallen force-pushed the macos_bringup_gallery_startup branch from 57b04c7 to 64d46cd Compare September 8, 2022 00:05

jmagman approved these changes Sep 8, 2022

View reviewed changes

a-wallen added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 8, 2022

auto-submit bot merged commit eac2329 into flutter:master Sep 8, 2022

a-wallen deleted the macos_bringup_gallery_startup branch September 8, 2022 16:49

This was referenced Sep 9, 2022

Roll Flutter from fc444aedab12 to ad946f8478bf (20 revisions) flutter/packages#2585

Merged

Roll Flutter from 66c306b45228 to ad946f8478bf (21 revisions) flutter/plugins#6378

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 9, 2022

eac2329 Re-land [macOS] Bring up "flutter_gallery" devicelab, start u…

9ebdce8

…p test for x86. (flutter/flutter#110379)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 9, 2022

eac2329 Re-land [macOS] Bring up "flutter_gallery" devicelab, start u…

5b3d08d

…p test for x86. (flutter/flutter#110379)

engine-flutter-autoroll mentioned this pull request Sep 9, 2022

Roll Flutter from 66c306b45228 to 4cd734ae3a1c (24 revisions) flutter/plugins#6379

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 9, 2022

eac2329 Re-land [macOS] Bring up "flutter_gallery" devicelab, start u…

db2b7b9

…p test for x86. (flutter/flutter#110379)

Conversation

a-wallen commented Aug 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reason

Related

Pre-launch Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a-wallen Aug 26, 2022 • 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!

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

Host-dependent tests (should not be moved)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a-wallen Aug 30, 2022 • 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!

Choose a reason for hiding this comment

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino commented Aug 29, 2022

Uh oh!

Uh oh!

a-wallen commented Sep 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

a-wallen commented Aug 26, 2022 •

edited

Loading

a-wallen Aug 26, 2022 •

edited

Loading

a-wallen Aug 30, 2022 •

edited

Loading

a-wallen commented Sep 8, 2022 •

edited

Loading