Skip to content

Conversation

@goderbauer
Copy link
Member

The widget under test already contains a MaterialApp, so there's no need to wrap it again with one in the test. In fact, the additional MaterialApp could hide problems in the widget under test.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels Jun 11, 2024
@goderbauer
Copy link
Member Author

@bleroux @TahaTesser Can I ask you to watch out for this pattern in reviews of API sample tests? If the widget under test already includes a MaterialApp we don't want the test to double-wrap it in another MaterialApp in the test. This can inadvertently hide problems in the API sample code (as an example see #150018).

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 11, 2024
@TahaTesser
Copy link
Member

TahaTesser commented Jun 11, 2024

@bleroux @TahaTesser Can I ask you to watch out for this pattern in reviews of API sample tests? If the widget under test already includes a MaterialApp we don't want the test to double-wrap it in another MaterialApp in the test. This can inadvertently hide problems in the API sample code (as an example see #150018).

Thanks for the reminder. We'll watch out for this.

@auto-submit auto-submit bot merged commit 962ae8a into flutter:master Jun 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 12, 2024
@goderbauer goderbauer deleted the doubleWrap branch June 12, 2024 16:20
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 12, 2024
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jun 14, 2024
The widget under test already contains a MaterialApp, so there's no need to wrap it again with one in the test. In fact, the additional MaterialApp could hide problems in the widget under test.
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jun 14, 2024
The widget under test already contains a MaterialApp, so there's no need to wrap it again with one in the test. In fact, the additional MaterialApp could hide problems in the widget under test.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants