refactor: remove material from color and image filter test#183563
Conversation
5b15b93 to
3216f65
Compare
There was a problem hiding this comment.
Code Review
This pull request successfully refactors color_filter_test.dart and image_filter_test.dart to remove dependencies on the Material library, replacing Material widgets with basic framework widgets and a test helper. The changes are correct and align with the goal of reducing cross-package imports in tests. I have two suggestions for improvement: one to add a const keyword for a performance gain, and another to address code duplication to enhance maintainability.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/flutter/test/widgets/color_filter_test.dart (47)
The Column widget and its descendants can be const. This is a good practice for performance as it allows Flutter to cache the widget and avoid unnecessary rebuilds. The Dart analyzer can help you add const where applicable.
home: const Column(References
- The repository style guide recommends following 'Effective Dart: Style' (line 20), which prefers using
constwith constructors that can beconstfor performance benefits. (link)
packages/flutter/test/widgets/image_filter_test.dart (138-169)
This TestWidgetsApp widget tree is a duplicate of the one used in the 'Image filter - matrix' test above. To improve maintainability and avoid code duplication, consider extracting this widget tree into a local helper widget or function.
References
- The repository style guide emphasizes optimizing for readability (line 29). Extracting duplicated code into a helper improves readability and maintainability, adhering to this principle. (link)
|
This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled. |
943db7a to
6a1787c
Compare
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
f028cad to
93d54b3
Compare
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Updated color for app bar. |
justinmc
left a comment
There was a problem hiding this comment.
Goldens approved and renewing my LGTM 👍
flutter/flutter@7245c3f...9cd60b5 2026-04-06 97480502+b-luk@users.noreply.github.com Fix go/ links in rbe.mde (flutter/flutter#184672) 2026-04-06 engine-flutter-autoroll@skia.org Roll Packages from d31df66 to 5299279 (2 revisions) (flutter/flutter#184663) 2026-04-06 1063596+reidbaker@users.noreply.github.com Add suggestion for what types of skills are helpful (flutter/flutter#184661) 2026-04-06 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from hrLZV-ij47FXnv_m5... to 1rcChbOv4nSTVkUxs... (flutter/flutter#184657) 2026-04-06 1063596+reidbaker@users.noreply.github.com Skill to find dart or flutter revision from a hash (flutter/flutter#184589) 2026-04-06 engine-flutter-autoroll@skia.org Roll Skia from 207c6721c64b to 163dfdf500c7 (7 revisions) (flutter/flutter#184650) 2026-04-05 engine-flutter-autoroll@skia.org Roll Skia from 52b79372764c to 207c6721c64b (1 revision) (flutter/flutter#184636) 2026-04-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from D2z-jMnRVbcwcraY-... to hrLZV-ij47FXnv_m5... (flutter/flutter#184623) 2026-04-05 engine-flutter-autoroll@skia.org Roll Skia from 500bc1651c86 to 52b79372764c (1 revision) (flutter/flutter#184621) 2026-04-04 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from PpL3Bn2YMb2h9LbdK... to D2z-jMnRVbcwcraY-... (flutter/flutter#184611) 2026-04-04 engine-flutter-autoroll@skia.org Roll Dart SDK from 102a3d1c7fe5 to 79f728f5d66e (1 revision) (flutter/flutter#184610) 2026-04-04 engine-flutter-autoroll@skia.org Roll Skia from 5e5a7f252b5e to 500bc1651c86 (1 revision) (flutter/flutter#184609) 2026-04-04 engine-flutter-autoroll@skia.org Roll Skia from ec209c206aa5 to 5e5a7f252b5e (2 revisions) (flutter/flutter#184606) 2026-04-04 engine-flutter-autoroll@skia.org Roll Dart SDK from f7be88117622 to 102a3d1c7fe5 (2 revisions) (flutter/flutter#184604) 2026-04-04 34465683+rkishan516@users.noreply.github.com refactor: remove material from color and image filter test (flutter/flutter#183563) 2026-04-04 engine-flutter-autoroll@skia.org Roll Skia from 50f3a9aaa185 to ec209c206aa5 (1 revision) (flutter/flutter#184601) 2026-04-03 engine-flutter-autoroll@skia.org Roll Skia from 4ecb7b28459f to 50f3a9aaa185 (3 revisions) (flutter/flutter#184599) 2026-04-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll ICU from ee5f27adc28b to ff7995a708a1 (5 revisions) (#184566)" (flutter/flutter#184586) 2026-04-03 15619084+vashworth@users.noreply.github.com Add integration scripts and tools for SwiftPM add to app (flutter/flutter#184204) 2026-04-03 engine-flutter-autoroll@skia.org Roll Dart SDK from 46f49142acd9 to f7be88117622 (1 revision) (flutter/flutter#184596) 2026-04-03 jesswon@google.com [AGP 9] Add Disable Built-in Kotlin and newDSL Migratorrs (flutter/flutter#184255) 2026-04-03 engine-flutter-autoroll@skia.org Roll Skia from 3b0f0f04f97c to 4ecb7b28459f (7 revisions) (flutter/flutter#184594) 2026-04-03 34465683+rkishan516@users.noreply.github.com refactor: remove material from list_view_viewporting_test and page_forward_transitions_test (flutter/flutter#183564) 2026-04-03 737941+loic-sharma@users.noreply.github.com [Dot shorthands] Migrate examples/api/lib/cupertino (flutter/flutter#183964) 2026-04-03 okorohelijah@google.com Downgrade CocoaPods doctor check to warning and fix build error messaging (flutter/flutter#184511) 2026-04-03 engine-flutter-autoroll@skia.org Roll Skia from 4134f8091147 to 3b0f0f04f97c (2 revisions) (flutter/flutter#184582) 2026-04-03 73785960+xfce0@users.noreply.github.com Remove live_text_utils cross-imports from material and cupertino tests (flutter/flutter#184517) 2026-04-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[data_assets] Cleanup tests (#184209)" (flutter/flutter#184575) 2026-04-03 engine-flutter-autoroll@skia.org Roll Packages from 66bf7ec to d31df66 (3 revisions) (flutter/flutter#184578) 2026-04-03 engine-flutter-autoroll@skia.org Roll Skia from 5d847ba5c4aa to 4134f8091147 (1 revision) (flutter/flutter#184573) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC bmparr@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…83563) This PR removes Material imports from color_filter_test and image_filter_test. part of: flutter#177415 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
This PR removes Material imports from color_filter_test and image_filter_test.
part of: #177415
Pre-launch Checklist
///).