Error message improvements #58204
Error message improvements #58204
Conversation
Hixie
commented
May 28, 2020
- Slightly improve error message for AlignmentDirectional.resolve without textDirection.
- Improve an error message for buildScope.
| await tester.pumpWidget(Container(child: _WidgetWithNoVisitChildren(_StatefulLeaf(key: key)))); | ||
| (key.currentState as _StatefulLeafState).markNeedsBuild(); | ||
| await tester.pumpWidget(Container()); | ||
| final dynamic exception = tester.takeException(); |
There was a problem hiding this comment.
I am surprised this is not allowed. What if there is an inherited widget that both container and _StatefulLeaf depend on. When the inherited widget mark both them dirty, the container rebuilt without the _StatefulLeaf?
There was a problem hiding this comment.
I'm not sure what you think should be allowed? Can you elaborate?
My intent was for this to test the error message that is reported as a result of a bad visitChildren.
There was a problem hiding this comment.
Oh Nvm, I was confused at the setup, I think this is fine. We should update the test title to be a bit more clear though.
| @@ -621,7 +621,7 @@ class _MixedAlignment extends AlignmentGeometry { | |||
|
|
|||
| @override | |||
| Alignment resolve(TextDirection direction) { | |||
| assert(direction != null); | |||
| assert(direction != null, 'Cannot resolve $runtimeType without a TextDirection.'); | |||
There was a problem hiding this comment.
Do we need test for this?
There was a problem hiding this comment.
The precise message? I can add a test if you like, sure. I hadn't added one for this because I wasn't particularly concerned about whether the message was changed.
There was a problem hiding this comment.
I think without a test is fine
| await tester.pumpWidget(Container(child: _WidgetWithNoVisitChildren(_StatefulLeaf(key: key)))); | ||
| (key.currentState as _StatefulLeafState).markNeedsBuild(); | ||
| await tester.pumpWidget(Container()); | ||
| final dynamic exception = tester.takeException(); |
There was a problem hiding this comment.
Oh Nvm, I was confused at the setup, I think this is fine. We should update the test title to be a bit more clear though.
| @@ -621,7 +621,7 @@ class _MixedAlignment extends AlignmentGeometry { | |||
|
|
|||
| @override | |||
| Alignment resolve(TextDirection direction) { | |||
| assert(direction != null); | |||
| assert(direction != null, 'Cannot resolve $runtimeType without a TextDirection.'); | |||
There was a problem hiding this comment.
I think without a test is fine
|
I'll update the test title and land on green. Thanks for the review! |
* Slightly improve error message for AlignmentDirectional.resolve without textDirection. * Improve an error message for buildScope.
|
I also added some more comments. |