Added a devicelab test for vulkan validation layers #134685

gaaclarke · 2023-09-13T20:11:57Z

This makes sure validation layers are being used and that there are no validation errors

fixes #134175

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.

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

gaaclarke · 2023-09-13T20:23:56Z

.ci.yaml

I'm not sure if we are guaranteed an arm64 device which we need for this test.

Is this supposed to run on an emulator?

I don't know of any reason why it shouldn't work on an arm64 emulator. We aren't changing the behavior of the renderer based on the capabilities of the device or anything, today at least.

I mean, is the desire to run this on hardware? Where do you want this to run?

It's probably safer to not run on the emulator for now. I turned that off and also switched this to a post build task since this is testing engine and framework integration, no need to gate framework prs on this.

Where do you want this to run?

I'm ambivalent, either works. I would suspect the emulator is easier on resources since it isn't gated on having a physical device. I moved it over since that way I could be reasonably sure we'd get arm64.

gaaclarke · 2023-09-13T20:26:31Z

This won't run successfully until flutter/engine#45777 has rolled.

zanderso · 2023-09-13T21:31:16Z

dev/devicelab/bin/tasks/hello_world_impeller.dart

When does startFlutter finish? Is that the point in time where the app is definitely running and displaying on the screen?

Nope, it's when the host process is executing.

Is 10 seconds enough time. Maybe for CI it makes sense to bump this up 3-5x?

10s was plenty of time locally, i increased it 3x. I looked through the rest of the code and didn't see a clear guidance about how long this should be.

zanderso · 2023-09-13T22:33:23Z

dev/devicelab/bin/tasks/hello_world_impeller.dart

Had another thought. Is it possible to watch the logs for the app to print a message indicating that it's time to stop the test?

Yea, but then you have the same problem, just in the flutter app not the test script. How long do you wait to send the message? Or what signal do you use to send it? Something like "pump and settle" might work. I'm not sure if that's available to us though. Maybe with a driver test?

If the problem we are trying to avoid is a test that is consistently reporting a false positives, it seems unlikely with a 30s timeout and a check for the backend message. Maybe it could miss once? I could make it wait for the backend error message indefinitely, then wait for errors for a certain amount of time after that. That would reduce the chance of a false positive.

From the perspective of the style guide, the app under test sending a message after N seconds is preferable to the test harness timing out after N seconds. In the former case, that's just how the app behaves and the test harness reacts appropriately. In the latter case, if the app behaves differently sometimes, but sometimes not, then the test would be flaky.

Okay, it is waiting indefinitely for the backend log now, PTAL. So, when we start waiting for any errors to pop up the engine is guaranteed to be engaged. There can be no false negatives from some increased latency in the setup of the test.

gaaclarke · 2023-09-13T23:20:00Z

(rebasing to test the positive case. for reference, the failing case can best tested off of 0aa8799)

…and there are no validation errors.

zanderso · 2023-09-14T00:37:41Z

dev/devicelab/bin/tasks/hello_world_impeller.dart

+        if (data.contains('Using the Impeller rendering backend')) {
+          // Sometimes more than one of these will be printed out if there is a
+          // fallback.
+          if (impellerBackendCount == 0) {


optional nit: You can use Completer.isCompleted.

https://api.dart.dev/dev/3.2.0-150.0.dev/dart-async/Completer/isCompleted.html

sgtm, done.

gaaclarke · 2023-09-14T15:30:35Z

@zanderso related to the thread on this PR and what we discussed in the sync yesterday I filed an issue about building a more complete validation test: #134738

…flutter#134685)

zanderso · 2023-09-15T17:58:17Z

.ci.yaml

      - bin/**
      - .ci.yaml

+  - name: Linux_android hello_world_impeller


Whoops. The Linux_android bit will make this run on a Moto G4.

Changing the Linux_android to Linux_pixel_7pro will make this run on a Pixel 7 Pro.

flutter/flutter@58ba6c2...72b69f9 2023-09-15 [email protected] Date picker dialog state should dispose members. (flutter/flutter#134804) 2023-09-15 [email protected] Roll Packages from 275b76c to bc8c2f2 (7 revisions) (flutter/flutter#134823) 2023-09-15 [email protected] Fix navigation rail hover misplaced when direction is RTL and extended is true (flutter/flutter#134815) 2023-09-15 [email protected] Applied the logo to the Discord badge. (flutter/flutter#134339) 2023-09-15 [email protected] Fix memory leak in ListWheelScrollView (flutter/flutter#134732) 2023-09-15 [email protected] Move two tests on Pixel 7 from staging to prod (flutter/flutter#134784) 2023-09-14 [email protected] Roll Flutter Engine from 683bca53d4d7 to 45bc4307cda3 (2 revisions) (flutter/flutter#134789) 2023-09-14 [email protected] [web] provide serviceWorkerVersion to the getNewServiceWorker function (flutter/flutter#131240) 2023-09-14 [email protected] Roll Flutter Engine from 3a3a2807c3b6 to 683bca53d4d7 (3 revisions) (flutter/flutter#134778) 2023-09-14 [email protected] Roll Flutter Engine from 035932d64017 to 3a3a2807c3b6 (5 revisions) (flutter/flutter#134769) 2023-09-14 [email protected] Allows page removal that contains Localhistoryentry (flutter/flutter#134757) 2023-09-14 [email protected] Roll Flutter Engine from e0b5b6c4eb76 to 035932d64017 (2 revisions) (flutter/flutter#134763) 2023-09-14 [email protected] Cover more test/widgets tests with leak tracking #3 (flutter/flutter#134576) 2023-09-14 [email protected] Roll Flutter Engine from 2cd34d23c1a2 to e0b5b6c4eb76 (2 revisions) (flutter/flutter#134755) 2023-09-14 [email protected] Set xcode version to older compatible version for microbenchmarks_ios_xcode_debug test (flutter/flutter#134693) 2023-09-14 [email protected] Cover some Services tests with leak tracing (flutter/flutter#134381) 2023-09-14 [email protected] Roll Flutter Engine from 4160ebacdae2 to 2cd34d23c1a2 (18 revisions) (flutter/flutter#134749) 2023-09-14 [email protected] ð�� Setup color tween for `RefreshIndicator` in a better way (flutter/flutter#134492) 2023-09-14 [email protected] Fix NavigationRail hover misplaced when using large icons (flutter/flutter#134719) 2023-09-14 [email protected] Roll Flutter Engine from cd90cc8469fb to 4160ebacdae2 (5 revisions) (flutter/flutter#134695) 2023-09-14 [email protected] Added a devicelab test for vulkan validation layers (flutter/flutter#134685) 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

…flutter#134685)

gaaclarke force-pushed the impeller-validation-test branch 3 times, most recently from 067a55e to ef5a969 Compare September 13, 2023 20:19

gaaclarke changed the title ~~Added a devicelab test for validation layers~~ Added a devicelab test for vulkan validation layers Sep 13, 2023

gaaclarke marked this pull request as ready for review September 13, 2023 20:21

gaaclarke force-pushed the impeller-validation-test branch from ef5a969 to 4662e06 Compare September 13, 2023 20:21

gaaclarke requested a review from zanderso September 13, 2023 20:21

gaaclarke commented Sep 13, 2023

View reviewed changes

zanderso reviewed Sep 13, 2023

View reviewed changes

gaaclarke requested a review from zanderso September 13, 2023 22:13

zanderso reviewed Sep 13, 2023

View reviewed changes

gaaclarke requested a review from zanderso September 13, 2023 22:53

gaaclarke added 6 commits September 13, 2023 16:20

Added a devicelab test to make sure validation layers are being used …

477c7f3

…and there are no validation errors.

updated ci / analysis fix

828bcca

increased timeout to 30s

31d7d9a

added comment

9f52ff2

added completor for the backend message

9bc5607

made sure we don't trigger the completer twice

9d6bebd

gaaclarke force-pushed the impeller-validation-test branch from 81e28d2 to 9d6bebd Compare September 13, 2023 23:20

removed stray print

b15950d

zanderso approved these changes Sep 14, 2023

View reviewed changes

zach feedback

bfe5c1a

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 14, 2023

gaaclarke mentioned this pull request Sep 14, 2023

[Impeller] Add gallery vulkan validation layers integration test #134738

Closed

gaaclarke merged commit 57ad4ff into flutter:master Sep 14, 2023

engine-flutter-autoroll mentioned this pull request Sep 15, 2023

Roll Flutter from 58ba6c295d8c to 0b540a87f1be (18 revisions) flutter/packages#4932

Closed

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

57ad4ff Added a devicelab test for vulkan validation layers (flutter/…

e85ce25

…flutter#134685)

engine-flutter-autoroll mentioned this pull request Sep 15, 2023

Roll Flutter from 58ba6c295d8c to 2a7ad01e559c (19 revisions) flutter/packages#4933

Closed

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

57ad4ff Added a devicelab test for vulkan validation layers (flutter/…

9605c21

…flutter#134685)

engine-flutter-autoroll mentioned this pull request Sep 15, 2023

Roll Flutter from 58ba6c295d8c to 72b69f944915 (21 revisions) flutter/packages#4934

Merged

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

57ad4ff Added a devicelab test for vulkan validation layers (flutter/…

0c9a3fd

…flutter#134685)

zanderso reviewed Sep 15, 2023

View reviewed changes

This was referenced Sep 15, 2023

moved hello_world_impeller to a 7pro #134830

Merged

Revert #46131, don't store vkImage, reset vkComandPool synchronously flutter/engine#46166

Merged

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

57ad4ff Added a devicelab test for vulkan validation layers (flutter/…

36121fc

…flutter#134685)

Added a devicelab test for vulkan validation layers #134685

Added a devicelab test for vulkan validation layers #134685

Uh oh!

Conversation

gaaclarke commented Sep 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

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

Uh oh!

gaaclarke commented Sep 13, 2023

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Sep 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Sep 14, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gaaclarke commented Sep 13, 2023 •

edited

Loading

gaaclarke commented Sep 13, 2023 •

edited

Loading