Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

various new_gallery__transition_perf devicelab tests timing out #114025

Closed
christopherfujino opened this issue Oct 25, 2022 · 14 comments
Closed

various new_gallery__transition_perf devicelab tests timing out #114025

christopherfujino opened this issue Oct 25, 2022 · 14 comments
Assignees
Labels
c: flake Tests that sometimes, but not always, incorrectly pass P0 Critical issues such as a build break or regression

Comments

@christopherfujino
Copy link
Member

christopherfujino commented Oct 25, 2022

For example:

[Linux_android new_gallery__transition_perf](https://ci.chromium.org/p/flutter/builders/prod/Linux_android%20new_gallery__transition_perf)

[Linux_samsung_s10 new_gallery__transition_perf](https://ci.chromium.org/p/flutter/builders/prod/Linux_samsung_s10%20new_gallery__transition_perf)

@christopherfujino christopherfujino added P2 and removed P2 labels Oct 25, 2022
@christopherfujino
Copy link
Member Author

Assigning P2 as it flakes infrequently enough that a re-run will usually fix the tree.

@zanderso
Copy link
Member

Comparing a successful vs. unsuccessful run of new_gallery__transition_perf, the driver commands and responses look different:

Unsuccessful: https://storage.googleapis.com/flutter_logs/flutter/b4058b95f52d461914b29c5dd2b5aa4b4ae09206/new_gallery__transition_perf/ee25924b-00d8-446a-a756-adcfd40f1687/flutter_driver_commands_0.log

2022-10-24 12:25:41.994213 >>> {command: request_data, message: isTestingCraneOnly}
2022-10-24 12:25:42.000725 <<< {isError: false, response: {message: false}, type: _extensionType, method: ext.flutter.driver}
2022-10-24 12:25:42.002498 >>> {command: request_data, message: isTestingReplyOnly}
2022-10-24 12:25:42.011756 <<< {isError: false, response: {message: false}, type: _extensionType, method: ext.flutter.driver}
2022-10-24 12:25:42.014435 >>> {command: waitFor, timeout: 5000, finderType: ByText, text: Gallery}
2022-10-24 12:25:42.019663 <<< {isError: false, response: {}, type: _extensionType, method: ext.flutter.driver}
2022-10-24 12:25:42.088070 >>> {command: waitFor, timeout: 10000, finderType: ByValueKey, keyValueString: reply@study, keyValueType: String}
2022-10-24 12:25:42.593312 >>> {command: scroll, finderType: ByValueKey, keyValueString: studyDemoList, keyValueType: String, dx: -800.0, dy: -50.0, duration: 100000, frequency: 60}
2022-10-24 12:25:42.598029 <<< {isError: false, response: {}, type: _extensionType, method: ext.flutter.driver}
2022-10-24 12:25:42.733032 <<< {isError: false, response: {}, type: _extensionType, method: ext.flutter.driver}
2022-10-24 12:25:43.238202 >>> {command: scrollIntoView, finderType: ByValueKey, keyValueString: reply@study, keyValueType: String, alignment: 0.5}
[EOF]

Successful: https://storage.googleapis.com/flutter_logs/flutter/563e0a4aae3640f0bee2bf41d8e6ce486b790e46/new_gallery__transition_perf/72e07505-6afb-45cf-9be5-a593b23575ea/flutter_driver_commands_0.log

2022-10-25 10:24:39.000071 >>> {command: request_data, message: isTestingCraneOnly}
2022-10-25 10:24:39.008162 <<< {isError: false, response: {message: false}, type: _extensionType, method: ext.flutter.driver}
2022-10-25 10:24:39.010004 >>> {command: request_data, message: isTestingReplyOnly}
2022-10-25 10:24:39.019209 <<< {isError: false, response: {message: false}, type: _extensionType, method: ext.flutter.driver}
2022-10-25 10:24:39.024503 >>> {command: waitFor, timeout: 5000, finderType: ByText, text: Gallery}
2022-10-25 10:24:39.072036 <<< {isError: false, response: {}, type: _extensionType, method: ext.flutter.driver}
2022-10-25 10:24:39.117994 >>> {command: waitFor, timeout: 10000, finderType: ByValueKey, keyValueString: reply@study, keyValueType: String}
2022-10-25 10:24:39.540429 <<< {isError: false, response: {}, type: _extensionType, method: ext.flutter.driver}
2022-10-25 10:24:39.624355 >>> {command: scrollIntoView, finderType: ByValueKey, keyValueString: reply@study, keyValueType: String, alignment: 0.5}
[Test continues...]

@zanderso
Copy link
Member

@jmagman recently updated the driver in flutter/gallery#792. @jmagman Do you know how to interpret the vm service messages and responses?

@jmagman
Copy link
Member

jmagman commented Oct 25, 2022

That sure does look similar to the logs in flutter/gallery#792. Unfortunately those Linux tests don't upload a screenshot on failure so it's not obvious why it's not scrolling into view. In the case of my PR I could see that the iPhone was in landscape which was a pretty big hint, then I could reproduce locally.

@jmagman
Copy link
Member

jmagman commented Oct 25, 2022

I'm not sure why it's not uploading screenshots for Linux, it should be...

Executing "/opt/s/w/ir/x/w/recipe_cleanup/tmpo5hlmkw8/flutter sdk/bin/flutter drive --screenshot /opt/s/w/ir/x/w/recipe_cleanup/flutter_logs_dir -...

@jmagman
Copy link
Member

jmagman commented Oct 25, 2022

In the other case FlutterDriverExtension timed out before the task was killed at 30 minutes, which gave it time to take the screenshot.

FlutterDriverExtension: Timeout while executing waitFor: TimeoutException after 0:00:10.000000: Future not completed

However here it's waiting the full 30 minutes and being killed at the task level, so doesn't have a chance to save the screenshot.

@zanderso
Copy link
Member

Assigning to @christopherfujino to look into the screenshots.

@christopherfujino
Copy link
Member Author

Since LUCI sends SIGTERM to the currently running process when timing it out, I will try registering a handler in the drive command to take a screenshot when SIGTERM is sent.

@christopherfujino
Copy link
Member Author

Since LUCI sends SIGTERM to the currently running process when timing it out, I will try registering a handler in the drive command to take a screenshot when SIGTERM is sent.

It appears this didn't work, so I will add a flag to allow explicitly specifying a timeout that will ensure the screenshot is taken.

@keyonghan keyonghan added the c: flake Tests that sometimes, but not always, incorrectly pass label Nov 1, 2022
cbracken added a commit to flutter/gallery that referenced this issue Nov 2, 2022
The gallery driver test includes logic to scroll through various lists
of demos in order to transition to each one. These lists include the
horizontally-scrolling studies list at the top of the gallery home page,
as well as the vertically-scrolling Material, Cupertino, and Other
demo lists on the same page.

TestDriver.scrollUntilVisible scrolls a parent list widget in increments
of dx and/or dy until the specified list item widget is visible. The
Studies carousel list at the top of the gallery home screen has scroll
physics that snap items to the starting edge of the widget. If the dx
scroll distance is too small, the Studies list doesn't scroll far enough
and snaps back to its original position. Conversely, if it scrolls too
far, it may scroll one widget too far and snap the next widget into
place. Instead, we now scroll 75% of the carousel width (empirically
determined) as an attempt at a value in the Goldilocks Zone.

Related: #792
Related: flutter/flutter#111131

Issue: flutter/flutter#114025
cbracken added a commit to flutter/gallery that referenced this issue Nov 2, 2022
The gallery driver test includes logic to scroll through various lists
of demos in order to transition to each one. These lists include the
horizontally-scrolling studies list at the top of the gallery home page,
as well as the vertically-scrolling Material, Cupertino, and Other
demo lists on the same page.

TestDriver.scrollUntilVisible scrolls a parent list widget in increments
of dx and/or dy until the specified list item widget is visible. The
Studies carousel list at the top of the gallery home screen has scroll
physics that snap items to the starting edge of the widget. If the dx
scroll distance is too small, the Studies list doesn't scroll far enough
and snaps back to its original position. Conversely, if it scrolls too
far, it may scroll one widget too far and snap the next widget into
place. Instead, we now scroll 75% of the carousel width (empirically
determined) as an attempt at a value in the Goldilocks Zone.

Related: #792
Related: flutter/flutter#111131

Issue: flutter/flutter#114025
cbracken added a commit to flutter/gallery that referenced this issue Nov 2, 2022
The gallery driver test includes logic to scroll through various lists
of demos in order to transition to each one. These lists include the
horizontally-scrolling studies list at the top of the gallery home page,
as well as the vertically-scrolling Material, Cupertino, and Other
demo lists on the same page.

TestDriver.scrollUntilVisible scrolls a parent list widget in increments
of dx and/or dy until the specified list item widget is visible. The
Studies carousel list at the top of the gallery home screen has scroll
physics that snap items to the starting edge of the widget. If the dx
scroll distance is too small, the Studies list doesn't scroll far enough
and snaps back to its original position. Conversely, if it scrolls too
far, it may scroll one widget too far and snap the next widget into
place. Instead, we now scroll 75% of the carousel width (empirically
determined) as an attempt at a value in the Goldilocks Zone.

Related: #792
Related: flutter/flutter#111131

Issue: flutter/flutter#114025
cbracken added a commit to flutter/gallery that referenced this issue Nov 2, 2022
The gallery driver test includes logic to scroll through various lists
of demos in order to transition to each one. These lists include the
horizontally-scrolling studies list at the top of the gallery home page,
as well as the vertically-scrolling Material, Cupertino, and Other
demo lists on the same page.

TestDriver.scrollUntilVisible scrolls a parent list widget in increments
of dx and/or dy until the specified list item widget is visible. The
Studies carousel list at the top of the gallery home screen has scroll
physics that snap items to the starting edge of the widget. If the dx
scroll distance is too small, the Studies list doesn't scroll far enough
and snaps back to its original position. Conversely, if it scrolls too
far, it may scroll one widget too far and snap the next widget into
place. Instead, we now scroll 75% of the carousel width (empirically
determined) as an attempt at a value in the Goldilocks Zone.

Related: #792
Related: flutter/flutter#111131

Issue: flutter/flutter#114025
cbracken added a commit to flutter/gallery that referenced this issue Nov 2, 2022
The gallery driver test includes logic to scroll through various lists
of demos in order to transition to each one. These lists include the
horizontally-scrolling studies list at the top of the gallery home page,
as well as the vertically-scrolling Material, Cupertino, and Other
demo lists on the same page.

TestDriver.scrollUntilVisible scrolls a parent list widget in increments
of dx and/or dy until the specified list item widget is visible. The
Studies carousel list at the top of the gallery home screen has scroll
physics that snap items to the starting edge of the widget. If the dx
scroll distance is too small, the Studies list doesn't scroll far enough
and snaps back to its original position. Conversely, if it scrolls too
far, it may scroll one widget too far and snap the next widget into
place. Instead, we now scroll 75% of the carousel width (empirically
determined) as an attempt at a value in the Goldilocks Zone.

Related: #792
Related: flutter/flutter#111131

Issue: flutter/flutter#114025
cbracken added a commit to cbracken/flutter_cocoon that referenced this issue Nov 2, 2022
dev/devicelab/lib/versions/gallery.dart contains the flutter/gallery
repo SHA to be synced during the gallery transitions test. Semantically,
it's similar to the engine's DEPS file or a pubspec.yaml in that updates
to it are version bumps.

This marks this file test exempt in our GitHub web hook.

Issue: flutter/flutter#114025
@cbracken
Copy link
Member

cbracken commented Nov 2, 2022

What I hope is a fix for this issue landed in flutter/gallery#824 and rolled to the framework in #114537.

Currently waiting to see how the next ~15-20 runs go.

@cbracken cbracken self-assigned this Nov 2, 2022
@cbracken
Copy link
Member

cbracken commented Nov 4, 2022

No flakes in the last 20 runs. I'm going to close this. @christopherfujino is there further work to do on the screenshotting side? If so, we should probably open a new (non-p2) bug dedicated to that work.

@cbracken cbracken closed this as completed Nov 4, 2022
cbracken added a commit to cbracken/flutter that referenced this issue Nov 4, 2022
These were marked flaky due to a timeout switching between items in the
Studies list at the top of the screen, which had snap-to-item scroll
physics.

The same flake also affected new_gallery_impeller__transition_perf which
was failing with the same flake, though was separately marked flaky due
to a separate (engine crash) flake documented in:
* flutter#112577
* flutter#112438

Leaving the latter marked flaky for the time being while that separate
issue is investigated.

Issue: flutter#114025
cbracken added a commit that referenced this issue Nov 6, 2022
These were marked flaky due to a timeout switching between items in the
Studies list at the top of the screen, which had snap-to-item scroll
physics.

The same flake also affected new_gallery_impeller__transition_perf which
was failing with the same flake, though was separately marked flaky due
to a separate (engine crash) flake documented in:
* #112577
* #112438

Leaving the latter marked flaky for the time being while that separate
issue is investigated.

Issue: #114025
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2022
@flutter-triage-bot flutter-triage-bot bot added P0 Critical issues such as a build break or regression and removed P2 labels Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: flake Tests that sometimes, but not always, incorrectly pass P0 Critical issues such as a build break or regression
Projects
None yet
Development

No branches or pull requests

5 participants