Fix Android `scenario_app` to actually run and block on CI #50414

matanlurey · 2024-02-06T21:13:59Z

20337ea (now reverted) intentionally re-introduced a bug (#49938) on a now working test runner, and verified that (with some fixes in this PR), we can now catch regressions on CI with SkiaGold:

ExternalTextureTests_testCanvasSurface.png

This test shows that what previously was a picture has been reduced down to a single stretched pixel.

BEFORE:

AFTER:

ExternalTextureTests_testRotatedMediaSurface_180.png

Similar to above, but shows another bug (kClamp versus kRepeat)

BEFORE:

AFTER:

/cc

This PR now makes the scenario_test useful and blocking for Android engine rolls:

Add support for connecting to SkiaGold by depending on goldctl in the appropriate configs.
Add some useful local debugging flags for skipping SkiaGold or running specific tests (not used by default).
Failing to connect to Skia gold is now a test failure.

flutter-dashboard · 2024-02-07T00:56:52Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #50414 at sha 84de0e4

This reverts commit 20337ea.

flutter-dashboard · 2024-02-07T01:45:08Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #50414 at sha eae0d9e

gaaclarke

LGTM! One more thing you'll want to do, not necessarily in this PR, is to assert that the files were created. The way our Skia Gold infrastructure works, it doesn't report an error if you fail to create the images. For impeller I just introduced that count. I think ideally there would be a text file with the name of all the images that should be generated.

gaaclarke · 2024-02-07T16:27:41Z

testing/scenario_app/bin/android_integration_tests.dart

      'smoke-test',
      help: 'runs a single test to verify the setup',
-      negatable: false,
+      valueHelp: 'empty to run dev.flutter.scenarios.EngineLaunchE2ETest, or specify a class',


Suggested change

valueHelp: 'empty to run dev.flutter.scenarios.EngineLaunchE2ETest, or specify a class',

defaultsTo: 'dev.flutter.scenarios.EngineLaunchE2ETest',

valueHelp: 'The class to execute.',

or

Suggested change

valueHelp: 'empty to run dev.flutter.scenarios.EngineLaunchE2ETest, or specify a class',

valueHelp: 'The class to execute, defaults to dev.flutter.scenarios.EngineLaunchE2ETest',

gaaclarke · 2024-02-07T16:31:45Z

testing/scenario_app/bin/android_integration_tests.dart

+        if (useSkiaGold) {
+          panic(<String>['skia gold client is unavailable']);
+        } else {
+          log('skia gold client is unavaialble');


Should this be a bit louder since they are requesting it but it will fail? "Error: Skia gold's client, goldctl, is unavailable"?

No. I added a clarification, this is in the non-CI case where it could be expected not to have Skia gold hooked up.

auto-submit · 2024-02-07T19:13:50Z

auto label is removed for flutter/engine/50414, due to - The status or check suite Linux Framework Smoke Tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2024-02-07T20:51:06Z

auto label is removed for flutter/engine/50414, due to - The status or check suite Linux Framework Smoke Tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

matanlurey · 2024-02-07T20:58:37Z

Merging on red due to flakes.

…lutter/engine#50414)

…143120) flutter/engine@e4a5acc...b1ba9f3 2024-02-07 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from kvEXgoydRYnW3UvX2... to CPiPDUwCrjHYf_lAR... (flutter/engine#50447) 2024-02-07 matanlurey@users.noreply.github.com Fix Android `scenario_app` to actually run and block on CI (flutter/engine#50414) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from kvEXgoydRYnW to CPiPDUwCrjHY If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

matanlurey added 3 commits February 6, 2024 12:11

Allow --smoke-test=full.test.Name

accbf55

++

1415747

Make sure CI breaks, as this is a bug.

20337ea

github-actions bot added the platform-android label Feb 6, 2024

matanlurey added 2 commits February 6, 2024 14:05

Pass GOLDCTL, make lack of skia gold a test failure.

1b411fc

++

84de0e4

flutter-dashboard bot added the will affect goldens label Feb 7, 2024

Revert "Make sure CI breaks, as this is a bug."

eae0d9e

This reverts commit 20337ea.

matanlurey changed the title ~~[DO NOT MERGE] Verify that an external texture bug will block presubmit (via Skia Gold)~~ Fix Android scenario_app to actually run and block on CI Feb 7, 2024

matanlurey requested review from johnmccutchan, dnfield and gaaclarke February 7, 2024 01:11

gaaclarke approved these changes Feb 7, 2024

View reviewed changes

Address feedback.

cf1bae2

matanlurey removed request for johnmccutchan and dnfield February 7, 2024 18:24

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2024

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2024

matanlurey merged commit 02263ca into flutter:main Feb 7, 2024
32 of 34 checks passed

matanlurey deleted the engine-scenario_app-androidTextureRepro branch February 7, 2024 20:58

engine-flutter-autoroll mentioned this pull request Feb 7, 2024

Roll Flutter Engine from e4a5acc14b44 to b1ba9f329f76 (2 revisions) flutter/flutter#143120

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 7, 2024

02263ca8b Fix Android scenario_app to actually run and block on CI (f…

9c75b96

…lutter/engine#50414)

matanlurey mentioned this pull request Feb 8, 2024

Add Impeller support to testing/scenario_app flutter/flutter#143171

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Android `scenario_app` to actually run and block on CI #50414

Fix Android `scenario_app` to actually run and block on CI #50414

matanlurey commented Feb 6, 2024 •

edited

Loading

flutter-dashboard bot commented Feb 7, 2024

flutter-dashboard bot commented Feb 7, 2024

gaaclarke left a comment

gaaclarke Feb 7, 2024

matanlurey Feb 7, 2024

gaaclarke Feb 7, 2024

matanlurey Feb 7, 2024

auto-submit bot commented Feb 7, 2024

auto-submit bot commented Feb 7, 2024

matanlurey commented Feb 7, 2024

	valueHelp: 'empty to run dev.flutter.scenarios.EngineLaunchE2ETest, or specify a class',
	defaultsTo: 'dev.flutter.scenarios.EngineLaunchE2ETest',
	valueHelp: 'The class to execute.',

	valueHelp: 'empty to run dev.flutter.scenarios.EngineLaunchE2ETest, or specify a class',
	valueHelp: 'The class to execute, defaults to dev.flutter.scenarios.EngineLaunchE2ETest',

Fix Android scenario_app to actually run and block on CI #50414

Fix Android scenario_app to actually run and block on CI #50414

Conversation

matanlurey commented Feb 6, 2024 • edited Loading

flutter-dashboard bot commented Feb 7, 2024

flutter-dashboard bot commented Feb 7, 2024

gaaclarke left a comment

Choose a reason for hiding this comment

gaaclarke Feb 7, 2024

Choose a reason for hiding this comment

matanlurey Feb 7, 2024

Choose a reason for hiding this comment

gaaclarke Feb 7, 2024

Choose a reason for hiding this comment

matanlurey Feb 7, 2024

Choose a reason for hiding this comment

auto-submit bot commented Feb 7, 2024

auto-submit bot commented Feb 7, 2024

matanlurey commented Feb 7, 2024

Fix Android `scenario_app` to actually run and block on CI #50414

Fix Android `scenario_app` to actually run and block on CI #50414

matanlurey commented Feb 6, 2024 •

edited

Loading