Support route on ios#99078
Conversation
flutter-dashboard
bot
added
the
tool
TESTOWNERS
Outdated
| /dev/devicelab/bin/tasks/platform_views_scroll_perf__timeline_summary.dart @zanderso @flutter/engine | ||
| /dev/devicelab/bin/tasks/plugin_dependencies_test.dart @jmagman @flutter/tool | ||
| /dev/devicelab/bin/tasks/routing_test.dart @zanderso @flutter/tool | ||
| /dev/devicelab/bin/tasks/route_test.dart @zanderso @flutter/tool |
There was a problem hiding this comment.
Does the renaming of this test file affect metrics? @keyonghan
There was a problem hiding this comment.
I think it's renaming the builder in the .ci.yaml that might be an issue: we're already running this as "Linux_android routing_test" today: https://ci.chromium.org/p/flutter/builders/prod/Linux_android%20routing_test.
There was a problem hiding this comment.
For metrics, are you referring to Skia perf metrics? If yes, then this file name change will affect the metrics. Skia perf metrics are now relying on task names, instead of builder names.
There was a problem hiding this comment.
Oh, I see. @Jasguerrero do we need to change the task name?
There was a problem hiding this comment.
@christopherfujino I don't think is necessary if it brings more problems than what it solves (just kind of confusing the task name is routing_test and the driver is route_test). I will be renaming it back to "routing_test"
| /dev/devicelab/bin/tasks/post_backdrop_filter_perf_ios__timeline_summary.dart @zanderso @flutter/engine | ||
| /dev/devicelab/bin/tasks/simple_animation_perf_ios.dart @zanderso @flutter/engine | ||
| /dev/devicelab/bin/tasks/tiles_scroll_perf_ios__timeline_summary.dart @zanderso @flutter/engine | ||
| /dev/devicelab/bin/tasks/routing_test.dart @zanderso @flutter/tool |
There was a problem hiding this comment.
This entry is not needed as we have already defined it in the linux section.
Could you please add a section like ## Linux android and Mac iOS Devicelab tests above the host only section, and add the shared entry there?
|
Looks like the engine change for this should be in the framework: #99446 |
keyonghan
left a comment
keyonghan
left a comment
There was a problem hiding this comment.
Thanks for the update! LGTM.
5 participants
#4703
This depend on an engine PR to merge first: flutter/engine#31555
Pre-launch Checklist
///).