Revamp the `scenario_app/../README.md` to encourage self-sustenance #51196

matanlurey · 2024-03-05T05:59:16Z

Feedback welcome, though if it is non-blocking and ambiguous consider a follow-up PR instead 😸

jonahwilliams

LGTM I read the whole thing

gmackall

LGTM, just a couple typo nits

gmackall · 2024-03-05T16:18:49Z

testing/scenario_app/android/README.md

-Or for a specific, build, such as `android_debug_unopt_arm64`:
+By default when run locally, the test runner:
+
+- Uses the engine-wide efault graphics backend for Android.


Suggested change

- Uses the engine-wide efault graphics backend for Android.

- Uses the engine-wide default graphics backend for Android.

gmackall · 2024-03-05T16:20:15Z

testing/scenario_app/android/README.md

+dart ./testing/scenario_app/bin/run_android_tests.dart --help
+```
+
+Frequency used options include:


Suggested change

Frequency used options include:

Frequently used options include:

fyi: there is a vscode plugin that will do spell checking, super handy.

gaaclarke

hoo boy, looks good. I just have some minor nits about what information should be at the top and generally there are a couple of cases where external links could be added or made more contextual.

gaaclarke · 2024-03-05T16:35:23Z

testing/scenario_app/android/README.md

+This test suite was [originally written in 2019](https://github.com/flutter/engine/pull/10007)
+with a goal of:
+
+> \[being\] suitable for embedders to do integration testing with - it has no
+> dependencies on the flutter_tools or framework, and so will not fail/flake
+> based on variances in those downstream.
+
+Unfortunately, the Android side of the test suite was never fully operational,
+and the tests, even if failing, were accidentally be reported as passing on CI.
+In 2024, as the team got closer to shipping our new graphics backend,
+[Impeller](https://docs.flutter.dev/perf/impeller) on Android, it was clear that
+we needed a reliable test suite for the engine on Android, particularly for
+visual tests around external textures and platform views.
+
+So, this package was revived and updated to be a (more) reliable test suite for
+the engine on Android. It's by no means complete
+([contributions welcome](#contributing)), but it did successfully catch at least
+one bug that would not have been detected automatically otherwise.
+
+_Go forth and test the engine on Android!_


I don't think this history-in-prose should get top billing. I think a Timeline section lower down would make more sense. We can keep the guiding principles.

Something like:

## Timeline - 2019 Originally implemented: lorem ipsom - 2024 Revamped to work for impeller.

gaaclarke · 2024-03-05T16:42:16Z

testing/scenario_app/android/README.md

+### Scope of Testing
+
+The tests in this package are end-to-end tests that _specifically_ exercise the
+engine and Android-specific embedding code. They are not unit tests for the


It would be nice to add a link after "unit tests" to the link provided below. Maybe using something like [1]

gaaclarke · 2024-03-05T16:43:38Z

testing/scenario_app/android/README.md

+  width="300"
+/>
+
+_The top picture is the Flutter app, and the bottom picture is just Android._


It would be nice to have a link to https://github.com/flutter/engine/tree/main/tools/compare_goldens here, maybe a "see also" note.

gaaclarke · 2024-03-05T16:45:29Z

testing/scenario_app/android/README.md

+dart ./testing/scenario_app/bin/run_android_tests.dart --help
+```
+
+Frequency used options include:


fyi: there is a vscode plugin that will do spell checking, super handy.

gaaclarke · 2024-03-05T16:46:38Z

testing/scenario_app/android/README.md

+
+See also:
+
+- [`DrawSolidBlueScreenTest.java`](./app/src/androidTest/java/dev/flutter/scenariosui/DrawSolidBlueScreenTest.java), an example of a JUnit test.


I'd put these links above to make them more contextual.

…stenance (flutter/engine#51196)

…144659) flutter/engine@effcf97...d374c78 2024-03-06 matanlurey@users.noreply.github.com Revamp the `scenario_app/../README.md` to encourage self-sustenance (flutter/engine#51196) 2024-03-06 jonahwilliams@google.com [Impeller] disable blending in gaussian intermediate steps. (flutter/engine#51118) 2024-03-06 bdero@google.com [Impeller] Enable depth buffer clipping & Stencil-then-Cover path rendering. (flutter/engine#51219) 2024-03-06 skia-flutter-autoroll@skia.org Roll Skia from 37947aec8c5c to 1d1fd7fe1a89 (1 revision) (flutter/engine#51216) 2024-03-06 bdero@google.com [Impeller] Fix path winding when bridging from contours with an odd number of points. (flutter/engine#51218) 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 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 March 4, 2024 21:20

Start on README rewrite.

0158b06

++

b7bc861

++

155525c

matanlurey requested review from dnfield, jonahwilliams, gaaclarke and gmackall March 5, 2024 05:59

jonahwilliams approved these changes Mar 5, 2024

View reviewed changes

gmackall approved these changes Mar 5, 2024

View reviewed changes

gaaclarke approved these changes Mar 5, 2024

View reviewed changes

++

d93aa74

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

auto-submit bot merged commit d374c78 into flutter:main Mar 6, 2024

engine-flutter-autoroll mentioned this pull request Mar 6, 2024

Roll Flutter Engine from effcf97a1f7c to d374c78bcf52 (5 revisions) flutter/flutter#144659

Merged

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

d374c78bc Revamp the scenario_app/../README.md to encourage self-su…

2081de8

…stenance (flutter/engine#51196)

	- Uses the engine-wide efault graphics backend for Android.
	- Uses the engine-wide default graphics backend for Android.

	Frequency used options include:
	Frequently used options include:


		See also:

		- [`DrawSolidBlueScreenTest.java`](./app/src/androidTest/java/dev/flutter/scenariosui/DrawSolidBlueScreenTest.java), an example of a JUnit test.

Revamp the scenario_app/../README.md to encourage self-sustenance #51196

Revamp the scenario_app/../README.md to encourage self-sustenance #51196

Uh oh!

Conversation

matanlurey commented Mar 5, 2024

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

gmackall left a comment

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 left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Revamp the `scenario_app/../README.md` to encourage self-sustenance #51196

Revamp the `scenario_app/../README.md` to encourage self-sustenance #51196