Migrate gallery ios tests to build+test #111164

keyonghan · 2022-09-08T03:37:08Z

Part of #103542

dev/devicelab/lib/tasks/gallery.dart

jmagman · 2022-09-19T21:17:42Z

.ci.yaml

@@ -3706,6 +3706,7 @@ targets:
      tags: >
        ["devicelab", "ios", "mac"]
      task_name: flutter_gallery__transition_perf_e2e_ios
+      artifact: gallery__transition_perf_e2e_ios


These artifacts can't be shared, flutter_gallery__transition_perf_e2e_impeller_ios it built with impeller and flutter_gallery__transition_perf_e2e_ios is not.

It seems like we should have some more generic way to do this, like hashing the example/integration_test app directory and the build flags? Also, some tests build the app multiple times with different flags (debug, profile, release).

Right, different tasks with different building args are not sharing these artifact.
I am planning to drop this artifact property for non-sharing ones. If two tasks do share the same artifact, we will specify the artifact explicitly.

Latest change: removed the artifact property here. In general, if artifact is not specified, task name will be used as artifact for the task. On the other side, if specified, the task will use it, and other task will share it if same artifact is defined.

This way, we have an assumption that each task builds a single artifact. This way, tasks that follow build+test+build+test etc. will not be supported for the later artifact sharing. As talked in go/devicelab-build-test, the later build should be covered in the test step.

Please let me know if this makes sense to you.

keyonghan · 2023-03-17T20:30:02Z

This needs to land after recipes support: https://flutter-review.googlesource.com/c/recipes/+/40582.

Success build based on this PR: https://ci.chromium.org/raw/build/logs.chromium.org/flutter/led/keyonghan_google.com/730d08ab49b99d961935edb719c7f4d5fc4dfc5873f78306e230545e66b5fbcf/+/build.proto?server=chromium-swarm.appspot.com

jmagman · 2023-03-22T00:06:16Z

.ci.yaml

+        ]
+      os: Mac-12
+      device_type: none
+      xcode: 14c18 # Xcode 14.2 to support iOS 16.2 in devicelab.


How are the recipes chosen? If it's devicelab this is fine since @godofredoc has said it has hermetic Xcode caches, and it seems like the Mac_build_test tests are going to need tethered devices. However if it's not the devicelab recipe then it should be

Suggested change

xcode: 14c18 # Xcode 14.2 to support iOS 16.2 in devicelab.

xcode: 14a5294e # xcode 14.0 beta 5

because of the Xcode cache issues: #117541 (comment)

@godofredoc can you clarify if this is right?

Good catch. The parent build is running on a host only bot, and should use same xcode as other host only targets. Updated to use 14a5294e and added comments to clarify

They need to be 14c18 if they are run on physical devices (which all of these are) since that version supports iOS 16.2, which is what the devices are running. I guess the individual builds need to override?
By the way if it wasn't for the Xcode caching issues #117541 (comment) I would make them all 14c18 instead of having different versions for different scenarios.

The devicelab_drone_build_test recipe currently injects the devicelab_osx_sdk version based on property xcode to run test. This will cause the test run on a devicelab bot but with an xcode version defined for the build running on the host only bot. This is clearly not expected.

Based on current logics (ci.yaml roller & cocoon scheduler) which auto generate targets' xcode (devicelab_osx_sdk or osx_sdk) based on the platform xcode, this PR as of now will not work.

With two xcode versions, we expect:

osx_sdk: 14a5294e for build step on the host only bot

devicelab_osx_sdk: 14c18 for test step on the devicelab bot.

We get:

same version for both.

To make this PR work, one way we can do is twist both the ci.yaml roller and cocoon scheduler to add both osx_sdk and devicelab_osx_sdk to the target. But this will add additional complexity to existing logics.

On the other hand, I do not see we will have a short-term fix on the xcode issue. To make things easier and unblock here. I am thinking to add clean cache flag to host only bots, allowing bots a couple of days to clean up old xcode's cache and prepare for the new version. This way, we will be unblocked here for all devicelab mitgration to build+test, and also make xcode version consistent across host only and devicelab targets.

With that being said, and considering ongoing frequent xcode changes, we should this to Q2.

@godofredoc before the xcode issue resolved from root (hopefully in Q2), do you have any objection for the above proposal?

This is unblocked, and have updated to use the latest xcode: 14e222b.

First run when no artifact exists: https://luci-milo.appspot.com/raw/build/logs.chromium.org/flutter/led/keyonghan_google.com/abc2f7c6ea7e36380cb59c73527839a5f35bd9ea5e51ce7c0d210fe704957246/+/build.proto
Second run when the artifact exists: https://luci-milo.appspot.com/raw/build/logs.chromium.org/flutter/led/keyonghan_google.com/5fe9cb5279aa53d509bb1cce94491cc5911bd9873851302b180e7205de244cb8/+/build.proto

@christopherfujino Please help take a look as @jmagman is ooo. Thanks!

.ci.yaml

dev/devicelab/lib/tasks/gallery.dart

flutter-dashboard · 2023-04-12T16:33:08Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

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

jmagman · 2023-05-01T23:46:27Z

Swapping out myself as a reviewer for @christopherfujino.

christopherfujino · 2023-05-09T23:42:17Z

@keyonghan is this ready for review?

keyonghan · 2023-05-09T23:50:36Z

@keyonghan is this ready for review?

Yes. Please help take a look. (sorry about the noise to restore the format)

christopherfujino · 2023-05-09T23:53:05Z

dev/devicelab/bin/tasks/flutter_gallery__transition_perf_e2e_ios.dart

  deviceOperatingSystem = DeviceOperatingSystem.ios;
-  await task(createGalleryTransitionE2ETest());
+  await task(createGalleryTransitionE2EBuildTest(args));


do we ever use these args?

We do. See GalleryTransitionBuildTest extends BuildTestTask, which parses these args. For context, this is how these args look like: https://flutter.googlesource.com/recipes/+/refs/heads/main/recipes/devicelab/devicelab_drone_build_test.py#142

I see. so we were never parsing these args (for this particular test) before?

That's correct. These args are used for build+test tasks only.

Btw: Linux gallery tasks have been running in prod for months, example: Linux_build_test flutter_gallery__transition_perf

christopherfujino

LGTM

…ets (#126482) This is a follow up of #111164. My local LED was based on `cpu: x86` dimension and `xcode` dependency. But these are missed in the above PR, which caused test failure: https://ci.chromium.org/ui/p/flutter/builders/staging/Mac_build_test%20flutter_gallery__transition_perf_e2e_ios/19/overview

flutter/flutter@8c5a1ea...a76dbe4 2023-05-10 [email protected] Manual roll Flutter Engine from 8ca16cba8c38 to 78f41a8f2f06 (2 revisions) (flutter/flutter#126392) 2023-05-10 [email protected] Roll flutter/packages to 0167d83 (flutter/flutter#126427) 2023-05-10 [email protected] Migrate gallery ios tests to build+test (flutter/flutter#111164) 2023-05-09 [email protected] Roll Flutter Engine from 824cd09b8c62 to 8ca16cba8c38 (5 revisions) (flutter/flutter#126360) 2023-05-09 [email protected] Revert "Provide default constraints for M3 dialogs" (flutter/flutter#126355) 2023-05-09 [email protected] Roll Packages from 4800d65 to 1f91710 (8 revisions) (flutter/flutter#126352) 2023-05-09 [email protected] [github] Add labeler action (flutter/flutter#126012) 2023-05-09 [email protected] Fix platformLocation path and search dropping (flutter/flutter#126232) 2023-05-09 [email protected] Roll Flutter Engine from 8d3a8162b3ab to 824cd09b8c62 (10 revisions) (flutter/flutter#126309) 2023-05-09 [email protected] Update FocusNode documentation (flutter/flutter#126331) 2023-05-09 [email protected] Remove dead code (flutter/flutter#126266) 2023-05-09 [email protected] fix AppBar's docs for backgroundColor (flutter/flutter#126194) 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 To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

#126941) Target `Mac_build_test flutter_gallery__transition_perf_e2e_ios` was enabled in staging: #111164, and it has passed more than 50 runs: https://ci.chromium.org/p/flutter/builders/staging/Mac_build_test%20flutter_gallery__transition_perf_e2e_ios?limit=50. Manually enabling it in prod and removing the old `Mac_ios flutter_gallery__transition_perf_e2e_ios`. The `Mac_build_test` one does the same thing as `Mac_ios` one, but separating build and test steps in separate targets. Context: #103542

…ets (flutter#126482) This is a follow up of flutter#111164. My local LED was based on `cpu: x86` dimension and `xcode` dependency. But these are missed in the above PR, which caused test failure: https://ci.chromium.org/ui/p/flutter/builders/staging/Mac_build_test%20flutter_gallery__transition_perf_e2e_ios/19/overview

flutter#126941) Target `Mac_build_test flutter_gallery__transition_perf_e2e_ios` was enabled in staging: flutter#111164, and it has passed more than 50 runs: https://ci.chromium.org/p/flutter/builders/staging/Mac_build_test%20flutter_gallery__transition_perf_e2e_ios?limit=50. Manually enabling it in prod and removing the old `Mac_ios flutter_gallery__transition_perf_e2e_ios`. The `Mac_build_test` one does the same thing as `Mac_ios` one, but separating build and test steps in separate targets. Context: flutter#103542

flutter/flutter@8c5a1ea...a76dbe4 2023-05-10 [email protected] Manual roll Flutter Engine from 8ca16cba8c38 to 78f41a8f2f06 (2 revisions) (flutter/flutter#126392) 2023-05-10 [email protected] Roll flutter/packages to 0167d83 (flutter/flutter#126427) 2023-05-10 [email protected] Migrate gallery ios tests to build+test (flutter/flutter#111164) 2023-05-09 [email protected] Roll Flutter Engine from 824cd09b8c62 to 8ca16cba8c38 (5 revisions) (flutter/flutter#126360) 2023-05-09 [email protected] Revert "Provide default constraints for M3 dialogs" (flutter/flutter#126355) 2023-05-09 [email protected] Roll Packages from 4800d65 to 1f91710 (8 revisions) (flutter/flutter#126352) 2023-05-09 [email protected] [github] Add labeler action (flutter/flutter#126012) 2023-05-09 [email protected] Fix platformLocation path and search dropping (flutter/flutter#126232) 2023-05-09 [email protected] Roll Flutter Engine from 8d3a8162b3ab to 824cd09b8c62 (10 revisions) (flutter/flutter#126309) 2023-05-09 [email protected] Update FocusNode documentation (flutter/flutter#126331) 2023-05-09 [email protected] Remove dead code (flutter/flutter#126266) 2023-05-09 [email protected] fix AppBar's docs for backgroundColor (flutter/flutter#126194) 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 To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Successful LED run based on flutter/flutter#111164 https://luci-milo.appspot.com/raw/build/logs.chromium.org/flutter/led/keyonghan_google.com/a43684afc067cba831ca3268c9621008cae1b916634886dada4c5c4a8c3d3587/+/build.proto Change-Id: Ia76e0593ddfa64939bca67ee3d68391b06ac558c Bug: flutter/flutter#103542 Reviewed-on: https://flutter-review.googlesource.com/c/recipes/+/40582 Reviewed-by: Godofredo Contreras <[email protected]> Commit-Queue: Keyong Han <[email protected]>

flutter-dashboard bot added the team Infra upgrades, team productivity, code health, technical debt. See also team: labels. label Sep 8, 2022

keyonghan marked this pull request as draft September 8, 2022 03:37

keyonghan commented Sep 13, 2022

View reviewed changes

dev/devicelab/lib/tasks/gallery.dart Outdated Show resolved Hide resolved

jmagman reviewed Sep 19, 2022

View reviewed changes

keyonghan force-pushed the gallery_ios branch from 9ae2d2d to 06217f7 Compare September 22, 2022 18:47

keyonghan force-pushed the gallery_ios branch from 06217f7 to d610761 Compare November 3, 2022 03:58

keyonghan force-pushed the gallery_ios branch 2 times, most recently from 75d06a0 to 802b641 Compare March 16, 2023 21:17

keyonghan force-pushed the gallery_ios branch from 802b641 to 5a6b5fa Compare March 17, 2023 20:28

keyonghan marked this pull request as ready for review March 17, 2023 20:30

keyonghan requested a review from godofredoc March 17, 2023 20:30

keyonghan force-pushed the gallery_ios branch from 5a6b5fa to 7e38f46 Compare March 21, 2023 23:41

jmagman reviewed Mar 22, 2023

View reviewed changes

keyonghan force-pushed the gallery_ios branch from 1312fbc to dedf70c Compare April 27, 2023 20:19

jmagman requested a review from christopherfujino May 1, 2023 23:46

keyonghan added 12 commits May 8, 2023 16:57

build+test

af0032d

add ios build args

831d4d1

update ios binary path

e444277

remove exclamation

60c4375

upload .app

cb21be1

test codesign

d9242df

back to no-codesign

ea39625

format

dd1c3a1

codesign

668a068

update target configs

30f9e34

xcode update + refactor binary path return

7a9349f

update to xcode 14c18

f6ba7db

keyonghan added 2 commits May 9, 2023 14:29

restore unrelated format changes

dff78bf

restore unrelated format changes

50795c3

christopherfujino reviewed May 9, 2023

View reviewed changes

christopherfujino approved these changes May 10, 2023

View reviewed changes

keyonghan added the autosubmit Merge PR when tree becomes green via auto submit App label May 10, 2023

auto-submit bot merged commit 8ab782d into flutter:master May 10, 2023

engine-flutter-autoroll mentioned this pull request May 10, 2023

Roll Flutter from 8c5a1ea7281a to df789c9e7609 (11 revisions) flutter/packages#3954

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

8ab782d Migrate gallery ios tests to build+test (flutter/flutter#111164)

ac4e3cc

engine-flutter-autoroll mentioned this pull request May 10, 2023

Roll Flutter from 8c5a1ea7281a to a76dbe44b99e (12 revisions) flutter/packages#3958

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

8ab782d Migrate gallery ios tests to build+test (flutter/flutter#111164)

bdc89b3

keyonghan mentioned this pull request May 10, 2023

Add cpu: x86 dimension and xcode dependency for Mac_build_test targets #126482

Merged

keyonghan mentioned this pull request May 16, 2023

Move Mac_build_test flutter_gallery__transition_perf_e2e_ios to prod #126941

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023

8ab782d Migrate gallery ios tests to build+test (flutter/flutter#111164)

3ee3411

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

8ab782d Migrate gallery ios tests to build+test (flutter/flutter#111164)

58174d2

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

8ab782d Migrate gallery ios tests to build+test (flutter/flutter#111164)

7ab9b44

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

8ab782d Migrate gallery ios tests to build+test (flutter/flutter#111164)

c6cb43c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate gallery ios tests to build+test #111164

Migrate gallery ios tests to build+test #111164

keyonghan commented Sep 8, 2022

jmagman Sep 19, 2022

keyonghan Sep 22, 2022

keyonghan Mar 17, 2023

keyonghan commented Mar 17, 2023

jmagman Mar 22, 2023

keyonghan Mar 22, 2023

jmagman Mar 22, 2023 •

edited

Loading

keyonghan Mar 23, 2023 •

edited

Loading

keyonghan May 9, 2023

flutter-dashboard bot commented Apr 12, 2023

jmagman commented May 1, 2023

christopherfujino commented May 9, 2023

keyonghan commented May 9, 2023

christopherfujino May 9, 2023

keyonghan May 10, 2023 •

edited

Loading

christopherfujino May 10, 2023 •

edited

Loading

keyonghan May 10, 2023

christopherfujino left a comment

	xcode: 14c18 # Xcode 14.2 to support iOS 16.2 in devicelab.
	xcode: 14a5294e # xcode 14.0 beta 5

Migrate gallery ios tests to build+test #111164

Migrate gallery ios tests to build+test #111164

Conversation

keyonghan commented Sep 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keyonghan commented Mar 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmagman Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

keyonghan Mar 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flutter-dashboard bot commented Apr 12, 2023

jmagman commented May 1, 2023

christopherfujino commented May 9, 2023

keyonghan commented May 9, 2023

Choose a reason for hiding this comment

keyonghan May 10, 2023 • edited Loading

Choose a reason for hiding this comment

christopherfujino May 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christopherfujino left a comment

Choose a reason for hiding this comment

jmagman Mar 22, 2023 •

edited

Loading

keyonghan Mar 23, 2023 •

edited

Loading

keyonghan May 10, 2023 •

edited

Loading

christopherfujino May 10, 2023 •

edited

Loading