-
Notifications
You must be signed in to change notification settings - Fork 6k
Fix scenario tests on Android #33360
Conversation
|
cc @dnfield |
dnfield
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned about adding screenshot tests this way. We need some kind of plan to make sure they won't be flaky and won't be used to cover things that we really need some non-golden test covering.
| For emulators running on a x64 host, build `android_debug_unopt_x64` using | ||
| `./tools/gn --android --unoptimized --goma --android-cpu=x64`. | ||
|
|
||
| Then, launch the emulator, and run `./testing/scenario_app/run_android_tests.sh android_debug_unopt_x64`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't introduce a shell script for this - use python.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(or Dart)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the existing shell script. It just calls dart with some flags. Is there a concern with this shell script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I'd just make sure it's actually used on CI. I think it's not, but if I'm wrong then leave it. If it's not then we should drop it.
@dnfield the idea is fetch the screenshots, and then send them to Skia Gold using https://github.com/flutter/engine/tree/main/web_sdk/web_test_utils. I don't know if it would flake, and if it flakes, I don't know why it would flake. What I'm thinking is to wait and see, and fix issues as they are discovered. I'm trying to be mindful about logging as much info as possible. |
dnfield
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nit about the color text output - we should try to detect if the terminal supports ansi and not output it, or detect if we're on LUCI and not output it.
Fixes the scenario tests on Android.
Issue: flutter/flutter#90401