-
Notifications
You must be signed in to change notification settings - Fork 29.8k
fix FlutterDriver timeout #31824
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
fix FlutterDriver timeout #31824
Conversation
|
@Hixie Would you review this ? |
eb84d1c to
df82b89
Compare
|
We should add a test for this. Otherwise someone might change this back and not realise it was intentional. |
|
@Hixie
|
|
|
||
| import 'common.dart'; | ||
|
|
||
| /// Default timeout value. |
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.
nit: document that this should correspond to _kUnusuallyLongTimeout in driver.dart.
Perhaps it would make sense to just let that constant become public with an @visibleForTesting on it. It doesn't seem like it should hurt anything to leak that as public to me.
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.
Thank you for review.
I will fix.
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.
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!
|
Deploy gallery failure is safe to ignore here, thanks for this! |
Description
FlutterDriver method's timeout is unexpected, I think.
example : waitFor, waitForAbsent, tap and so on.
If I pass timeout argument less than 5 seconds (ex: 3 seconds), I expect waitFor method times out in 3 seconds. But, waitFor mathod times out in 5 seconds.
→ I changed the behavior. So, now, the behavior matches document.
Related Issues
Tests
I added the test to ensure that custom timeout works.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?