-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Remove divider from widgets tests #180999
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.
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
Remove divider from widgets tests #180999
Conversation
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.
Code Review
This pull request effectively removes usages of Divider from several widget tests to reduce dependencies on the material package. The changes correctly replace Divider with SizedBox or custom widget implementations as appropriate. A new file, widget_test_utils.dart, is introduced to provide basic widgets for tests, which is a good step towards better test modularity. The changes are well-executed and align with the PR's goal. I have one suggestion for the new test utility file to improve clarity and maintainability.
| /// Get a random color for use in a widget test. | ||
| /// | ||
| /// The returned color will be fully opaque, | ||
| /// but the [Color.r], [Color.g], and [Color.b] channels | ||
| /// will each be randomized between `00` and `FF`. | ||
| Color getRandomTestColor(int index) { |
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.
The function name getRandomTestColor and its documentation are misleading. The function returns a deterministic color from a predefined list based on the index, not a random color. The documentation also incorrectly states that RGB channels are randomized.
To improve clarity and accuracy, I suggest renaming the function to getTestColor and updating the documentation to reflect its actual behavior. This aligns with the style guide's emphasis on readability and useful documentation.
/// Get a deterministic color for use in a widget test.
///
/// The returned color will be fully opaque and is selected from a predefined
/// list of colors based on the given [index].
Color getTestColor(int index) {b8ee04d to
7ab2ade
Compare
justinmc
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.
LGTM 👍
|
|
||
| // Regression test for https://github.com/flutter/flutter/issues/100451 | ||
| testWidgets('ListView.separator respects findChildIndexCallback', (WidgetTester tester) async { | ||
| testWidgets('ListView.separated respects findChildIndexCallback', (WidgetTester tester) async { |
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.
Good catch.
|
Since this has 2 approvals now, can this get an autosubmit as well? |
This PR removes
Dividerusages fromtest/widgets/scroll_view_test.dartandtest/widgets/list_view_test.dart.Usages of
Divider()still exist underexamples/apiand in code samples underlib/widgets, though.Part of #177415
Part of #180501
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.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.