Skip to content

Conversation

@dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Jul 29, 2021

This is a reattempt to fix #87194.

The previous attempt #87240 was reverted because many internal tests was written in a way that sets the view configuration at the beginning of a file and expect to use it throughout the file, which was broken by the previous method.

void main() {
  binding.renderView.configuration = TestViewConfiguration(size: const Size(900, 900));
  testWidget('test1', ...);
  testWidget('test2', ...);
}

This PR restores the surface size and the view configuration in the following strategy:

  • The surface size is always reset to null at postTest, since the surface size can not be changed out of tests.
  • The view configuration is recorded at reset (the pre-test hook), and is restored to that value at postTest.

I tested this change against some previously failing tests and they have been passing.

This PR also removes the temporary teardown cleanup in viewport_test.dart.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@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 29, 2021
@google-cla google-cla bot added the cla: yes label Jul 29, 2021
@dkwingsmt dkwingsmt requested a review from goderbauer July 31, 2021 02:38
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

nit: remove this blank line?

Copy link
Member

Choose a reason for hiding this comment

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

nit: remove a blank line?

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

(Probably needs a rebase to make the check happy)

@dkwingsmt dkwingsmt force-pushed the reland-restore-surface-size branch from 42b50fa to f9a0be8 Compare August 4, 2021 03:07
@dkwingsmt dkwingsmt merged commit 91f8b6b into flutter:master Aug 11, 2021
@dkwingsmt dkwingsmt deleted the reland-restore-surface-size branch August 11, 2021 07:38
dkwingsmt added a commit that referenced this pull request Aug 16, 2021
dkwingsmt added a commit that referenced this pull request Aug 16, 2021
blasten pushed a commit to blasten/flutter that referenced this pull request Aug 19, 2021
dkwingsmt added a commit to dkwingsmt/flutter that referenced this pull request Sep 8, 2021
dkwingsmt added a commit to dkwingsmt/flutter that referenced this pull request Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

widget_tester_live_device_test.dart: Should print message on pointer events Failing consistently

2 participants