Skip to content

[material_ui] Port flutter/flutter #186777 MaterialColor test - #12858

Merged
auto-submit[bot] merged 4 commits into
flutter:mainfrom
MarlonJD:port-flutter-186777-material-colors
Oct 22, 2026
Merged

[material_ui] Port flutter/flutter #186777 MaterialColor test#12858
auto-submit[bot] merged 4 commits into
flutter:mainfrom
MarlonJD:port-flutter-186777-material-colors

Conversation

@MarlonJD

@MarlonJD MarlonJD commented Sep 13, 2026 •
edited by navaronbracke
Loading

Copy link
Copy Markdown
Contributor

This PR ports the Material-specific test from flutter/flutter#186777 to material_ui.

The MaterialColor swatch comparison test verifies equality for separate swatch maps containing the same standard Color values.

This is a test-only port, so no package version or changelog entry is needed.

Part of flutter/flutter#188444.

The PR that removes the test in the framework is flutter/flutter#192716

@github-actions github-actions Bot added p: material_ui triage-design Should be looked at in design triage labels Sep 13, 2026
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 13, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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 adds a new test case to verify MaterialColor swatch comparison and equality. The reviewer suggested formatting the map literals to comply with the 80-character limit and standard dart format rules, and recommended adding an assertion to verify that equal objects produce identical hash codes.

Comment thread packages/material_ui/test/colors_test.dart
Comment on lines +88 to +94
final sampleMap = <int, Color>{50: const Color(0xFFE1F5FE), 100: const Color(0xFFB3E5FC)};
final anotherMap = <int, Color>{50: const Color(0xFFE1F5FE), 100: const Color(0xFFB3E5FC)};
final first = MaterialColor(0, sampleMap);
final second = MaterialColor(0, sampleMap);
final third = MaterialColor(0, anotherMap);
expect(first, second);
expect(first, third);

@gemini-code-assist gemini-code-assist Bot Sep 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

There are two improvements we can make here:

  1. Formatting: The map initialization lines exceed the 80-character limit and do not adhere to the standard dart format formatting rules. Running dart format will automatically wrap these map literals. Formatting them to span multiple lines will ensure style consistency and prevent potential CI formatting check failures.
  2. Test Coverage: When testing custom equality (operator ==), it is a best practice to also verify that equal objects produce identical hash codes (hashCode). Adding an assertion for hashCode ensures the contract of operator == and hashCode is fully verified.
    final sampleMap = <int, Color>{
      50: const Color(0xFFE1F5FE),
      100: const Color(0xFFB3E5FC),
    };
    final anotherMap = <int, Color>{
      50: const Color(0xFFE1F5FE),
      100: const Color(0xFFB3E5FC),
    };
    final first = MaterialColor(0, sampleMap);
    final second = MaterialColor(0, sampleMap);
    final third = MaterialColor(0, anotherMap);
    expect(first, second);
    expect(first, third);
    expect(first.hashCode, third.hashCode);
References
  1. Code should follow the relevant style guides, and use the correct auto-formatter, for each language, as described in the repository contributing guide's Style section. (link)

@MarlonJD MarlonJD mentioned this pull request Sep 13, 2026
Open
Renzo-Olivares
Renzo-Olivares approved these changes Sep 24, 2026
View reviewed changes

@Renzo-Olivares Renzo-Olivares left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@elliette
elliette self-requested a review September 29, 2026 22:07
@elliette elliette added the CICD Run CI/CD label Sep 30, 2026
@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Sep 30, 2026
@elliette elliette added autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD and removed autosubmit Merge PR when tree becomes green via auto submit App labels Sep 30, 2026
elliette
elliette requested changes Sep 30, 2026
View reviewed changes

@elliette elliette left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @MarlonJD - looks like the analyze step is failing:

info - test/colors_test.dart:88:11 - Omit the type annotation on a local variable when the type is obvious. Try removing the type annotation. - omit_obvious_local_variable_types
   info - test/colors_test.dart:92:11 - Omit the type annotation on a local variable when the type is obvious. Try removing the type annotation. - omit_obvious_local_variable_types

Could you please fix? Thanks!

@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Sep 30, 2026
@navaronbracke navaronbracke added the CICD Run CI/CD label Oct 5, 2026
@QuncCccccc
QuncCccccc requested a review from elliette October 6, 2026 21:20
@QuncCccccc QuncCccccc added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 6, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 6, 2026
@auto-submit

auto-submit Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/packages/12858, because This PR has not met approval requirements for merging. Changes were requested by {elliette}, please make the needed changes and resubmit this PR.
The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a member of flutter-hackers before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@elliette elliette added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 22, 2026
@auto-submit
auto-submit Bot merged commit 93baed9 into flutter:main Oct 22, 2026
14 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 22, 2026
Merged
GhagSagar23 pushed a commit to Kryonex-Labs/flutter that referenced this pull request Oct 22, 2026
@engine-flutter-autoroll
Roll Packages from 1383427 to 93baed9 (9 revisions) (flutter#195068)
8059aa4 
flutter/packages@1383427...93baed9

2026-10-22 [email protected] [material_ui] Port flutter/flutter
flutter#186777 MaterialColor test (flutter/packages#12858)
2026-10-21 [email protected] [rfw] Migrate to material_ui
(flutter/packages#12811)
2026-10-21 [email protected] [cupertino_ui] Fix the installing
command of the cupertino_ui package (flutter/packages#13141)
2026-10-21 [email protected]
[cupertino_ui] Unskip web tests that pass on CanvasKit and Skwasm:
date_picker_test, spell_check_suggestions_toolbar_test
(flutter/packages#13367)
2026-10-21 [email protected] [video_player] Convert player classes
to Swift (flutter/packages#13197)
2026-10-21 [email protected] [camera_windows] Fall back to a physical
stream for cameras that reject preferred-stream selectors
(flutter/packages#12748)
2026-10-21 [email protected] [google_sign_in_ios, video_player] Specify
types for pattern variable declarations (flutter/packages#13362)
2026-10-21 [email protected] [google_maps_flutter_web] Issue 64073
implement my location (flutter/packages#12186)
2026-10-21 [email protected] [material_ui]
Unskip web tests that pass on CanvasKit and Skwasm: app_bar_theme_test,
input_decorator_test, paginated_data_table_test,
spell_check_suggestions_toolbar_test (flutter/packages#13352)

If this roll has caused a breakage, revert this CL and set the roller
to dry run mode using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
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
suryaKommana2662 pushed a commit to suryaKommana2662/flutter that referenced this pull request Oct 29, 2026
@engine-flutter-autoroll @suryaKommana2662
Roll Packages from 1383427 to 93baed9 (9 revisions) (flutter#195068)
8150ebe 
flutter/packages@1383427...93baed9

2026-10-22 [email protected] [material_ui] Port flutter/flutter
flutter#186777 MaterialColor test (flutter/packages#12858)
2026-10-21 [email protected] [rfw] Migrate to material_ui
(flutter/packages#12811)
2026-10-21 [email protected] [cupertino_ui] Fix the installing
command of the cupertino_ui package (flutter/packages#13141)
2026-10-21 [email protected]
[cupertino_ui] Unskip web tests that pass on CanvasKit and Skwasm:
date_picker_test, spell_check_suggestions_toolbar_test
(flutter/packages#13367)
2026-10-21 [email protected] [video_player] Convert player classes
to Swift (flutter/packages#13197)
2026-10-21 [email protected] [camera_windows] Fall back to a physical
stream for cameras that reject preferred-stream selectors
(flutter/packages#12748)
2026-10-21 [email protected] [google_sign_in_ios, video_player] Specify
types for pattern variable declarations (flutter/packages#13362)
2026-10-21 [email protected] [google_maps_flutter_web] Issue 64073
implement my location (flutter/packages#12186)
2026-10-21 [email protected] [material_ui]
Unskip web tests that pass on CanvasKit and Skwasm: app_bar_theme_test,
input_decorator_test, paginated_data_table_test,
spell_check_suggestions_toolbar_test (flutter/packages#13352)

If this roll has caused a breakage, revert this CL and set the roller
to dry run mode using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Renzo-Olivares Renzo-Olivares Renzo-Olivares approved these changes
@elliette elliette elliette approved these changes
@navaronbracke navaronbracke navaronbracke approved these changes
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD p: material_ui triage-design Should be looked at in design triage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@MarlonJD @Renzo-Olivares @elliette @navaronbracke @QuncCccccc