Skip to content

Restore the default surface size in TestWidgetsFlutterBinding.reset #87226

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.

Sign up for GitHub

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

Jump to bottom
Closed
jason-simmons wants to merge 1 commit into from
Closed

Restore the default surface size in TestWidgetsFlutterBinding.reset #87226

jason-simmons wants to merge 1 commit into from

Conversation

@jason-simmons
Copy link
Member

@jason-simmons jason-simmons commented Jul 28, 2021

See #87194

@jason-simmons jason-simmons requested review from dkwingsmt and flar July 28, 2021 21:16
@flutter-dashboard flutter-dashboard bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Jul 28, 2021
@google-cla google-cla bot added the cla: yes label Jul 28, 2021
flar
flar approved these changes Jul 28, 2021
View reviewed changes
Copy link
Contributor

@flar flar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't look too closely at the test change as I'm assuming that the change was a simple revert to the previous version without the try/finally.

LGTM.

@dkwingsmt
Copy link
Contributor

dkwingsmt commented Jul 28, 2021

I'm concerned about the side effect introduced by setSurfaceSize. Maybe we just need _surfaceSize = null? It might be the cause of the current red checks.

@jason-simmons
Copy link
Member Author

jason-simmons commented Jul 28, 2021

Updated this to set _surfaceSize = null followed by a synchronous call to handleMetricsChanged

@flar flar mentioned this pull request Jul 28, 2021
Merged
8 tasks
@jason-simmons
Copy link
Member Author

jason-simmons commented Jul 28, 2021

Just doing _surfaceSize = null is insufficient to update binding state that is derived from the surface size.

But calling handleMetricsChanged in reset() is apparently breaking other tests (e.g. flutter_driver tests that are affected by task scheduling behavior)

Not sure if there is any safe way to restore this state during reset()

@dkwingsmt
Copy link
Contributor

dkwingsmt commented Jul 29, 2021

I made another attempt of this PR at #87240 and it passes. Apparently the problem is that renderView was updated with the new surface size.

@dkwingsmt dkwingsmt removed their request for review September 10, 2021 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@flar flar flar approved these changes

Assignees

No one assigned

Labels

a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jason-simmons @dkwingsmt @flar