promote WidgetTester pumpAndSettle #62640
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
flutter-dashboard
bot
added
a: tests
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
By breaking change I mean change the order argument But of course I can avoid breaking change by removing |
There was a problem hiding this comment.
Throwing an exception when there's no back button still seems to be desired. As expectSync may not be available from WidgetController, maybe we can use something else (e.g., FlutterError). That way, we can probably still preserve the fails when there are no back buttons test with a small modification.
There was a problem hiding this comment.
I wonder if it really is desired/consistent with other APIs. By removing the pageBack method, IDE reminds me that the matcher.dart is no longer needed: basically this method is the only method expecting the existence of a finder, while all other methods are assuming a finder exists, and rely on when really evaluating a finder to raise an error.
There was a problem hiding this comment.
this whole method is a bit dubious to be honest (we really shouldn't be depending on material or cupertino in flutter_test). It's all just convenience methods. So once we have them anyway, why not also make sure there's a back button.
That said, whether we throw a FlutterError or a TestError (or whatever expectSync fires) doesn't seem to matter that much. This is a test framework, so expectSync is not unreasonable IMHO.
There was a problem hiding this comment.
as it is you're still going to through when you try to tap a the button and there isn't one
|
So I reverted the change of |
CareF
changed the title
|
@liyuqian I'm not sure if there is any non-public google3 test we need to try to confirm it is not a break test in sense of https://github.com/flutter/flutter/wiki/Tree-hygiene#1-determine-if-your-change-is-a-breaking-change |
|
@CareF I started the Google internal tests in cl/324724889. You may not have access to it so I'll help you check it. The results should be in tomorrow morning so we can go through it by sharing my screen in our 1:1 meeting. |
liyuqian
left a comment
liyuqian
left a comment
There was a problem hiding this comment.
LGTM. This approval should also trigger the "Google testing" check. It should check whether this breaks any Google internal tests.
5 participants
Description
This is part of #62064 to promote pumpAndSettle from
WidgetTestertoWidgetController.And as part of the style guide, I removed
timeoutoptional argument.Related Issues
#61511
Tests
I added
'Test pump on LiveWidgetController'inpackages/flutter_test/test/live_widget_controller_test.dart.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
Did any tests fail when you ran them? Please read Handling breaking changes.