-
Notifications
You must be signed in to change notification settings - Fork 1.6k
refactor: migrate test_benchmarks to null safety #757
Conversation
| @required Rect large, | ||
| @required Rect small, | ||
| @required AxisDirection axisDirection, | ||
| bool? _hasSufficientFreeRoom({ |
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.
It doesn't look like the call to this handles it potentially being null, so I'd make it non-nullable, and then in line 43 below give it an initial value (either true or false) before the switch statement.
|
Can't figure out the failure, meanwhile closing it for anyone else to have a look. |
|
Re-opening to see/fix the failures |
|
@guidezpl I can't make sense of this error. Do you understand what's going on? |
Taking a look now. |
|
Thank you for your message! I tested the web benchmarks a few times locally, and it worked every time after updating the package versions. A similar PR #768 passes all checks. Have you tried re-running the web benchmark checks? Does the failure persist? |
|
I've tried several re-runs over several days. I just did it again, this time selecting to include debug info. But I still don't see anything that helps me understand what's going on: |
87eea15 to
787a122
Compare
guidezpl
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.
Nice. I'll fix the goldens issue separately (the banner divider changed color) after the change is rolled in
Migrate test_benchmarks to null safety
List which issues are fixed by this PR. You must list at least one issue.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.