-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix flaky complex_layout_scroll_perf__memory test #150287
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
Conversation
Initial tap is missing sometimes; either its never delivered or it is delivered before gesture controller is hooked up. 1: Update the two perf tests to output when TAPPED is received 2: Update the MemoryTest to keep tapping while waiting for TAPPED Tested on devicelab: * setting iterations=1 * removing the timeout before READY * running tests in a while loop Before this change, you could get the test to hang often. After this change you'll see "tapping device... [x]" where x is the counter. Fixes flutter#150096
gaaclarke
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 modulo single character variable name.
Instead of repeatedly tapping, it seems like you could do the same thing you did to receive the tap event to know when the thing is ready to be tapped. Seems like there should be a latch for that in Flutter? Maybe sending the message when the State is initialized would be enough?
There is a risk of accidentally tapping twice before registering the tap because of some sort of latency in receiving the TAPPED print statement. Hopefully 250ms makes that unlikely though and is definitely better than what we had.
Co-authored-by: gaaclarke <[email protected]>
I think that would be a moving target. If there was a magical "we're ready for input" then I'd wait on it; but a side effect of state loaded might be undocumented.
I could increase the caffeine to 100ms tapping if that makes things better. We could also delay after the READY for ~milliseconds. |
|
The other tests don't send the "TAPPED" signal, so of course they timeout. |
|
A reason for requesting a revert of flutter/flutter/150287 could |
|
Reason for revert: other memperf tests don't wait for or send a TAPPED; so they fail. |
This reverts commit 499c84c.
…#150293) Reverts: #150287 Initiated by: jtmcdole Reason for reverting: other memperf tests don't wait for or send a TAPPED; so they fail. Original PR Author: jtmcdole Reviewed By: {gaaclarke} This change reverts the following previous change: Initial tap is missing sometimes; either its never delivered or it is delivered before gesture controller is hooked up. 1: Update the two perf tests to output when TAPPED is received 2: Update the MemoryTest to keep tapping while waiting for TAPPED Tested on devicelab: * setting iterations=1 * removing the timeout before READY * running tests in a while loop Before this change, you could get the test to hang often. After this change you'll see "tapping device... [x]" where x is the counter. Fixes #150096
flutter/flutter@349ec71...5187cab 2024-06-15 [email protected] Roll Flutter Engine from 8167dffd1914 to 9779c273aac3 (21 revisions) (flutter/flutter#150290) 2024-06-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Fix flaky complex_layout_scroll_perf__memory test (#150287)" (flutter/flutter#150293) 2024-06-15 [email protected] Fix flaky complex_layout_scroll_perf__memory test (flutter/flutter#150287) 2024-06-14 [email protected] Make `CupertinoTextField` respect decoration color when disabled (flutter/flutter#149774) 2024-06-14 [email protected] Add transparent color to Cupertino colors (flutter/flutter#150149) 2024-06-14 [email protected] Roll pub packages (flutter/flutter#150267) 2024-06-14 [email protected] Roll Packages from 7805455 to dd04ab1 (4 revisions) (flutter/flutter#150264) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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
Initial tap is missing sometimes; either its never delivered or it is delivered before gesture controller is hooked up.
1: Update the two perf tests to output when TAPPED is received
2: Update the MemoryTest to keep tapping while waiting for TAPPED
Tested on devicelab:
Before this change, you could get the test to hang often. After this change you'll see "tapping device... [x]" where x is the counter.
Fixes #150096