Move Material color tests to material by MarlonJD · Pull Request #186777 · flutter/flutter

MarlonJD · 2026-05-20T01:54:12Z

Summary

Moves the Material color and palette tests from painting into the Material test directory:

packages/flutter/test/painting/colors_test.dart -> packages/flutter/test/material/colors_test.dart

Removes the now-obsolete painting cross-import allowlist entry.

Part of #177414
Refs #177028

Tests

bin/cache/dart-sdk/bin/dart --enable-asserts dev/bots/check_tests_cross_imports.dart
bin/cache/dart-sdk/bin/dart analyze packages/flutter/test/material/colors_test.dart dev/bots/check_tests_cross_imports.dart
bin/cache/dart-sdk/bin/dart format --output=none --set-exit-if-changed packages/flutter/test/material/colors_test.dart dev/bots/check_tests_cross_imports.dart
git diff --check
./bin/flutter analyze --no-pub packages/flutter/test/material/colors_test.dart dev/bots/check_tests_cross_imports.dart
./bin/flutter test --no-pub packages/flutter/test/material/colors_test.dart

gemini-code-assist

Code Review

This pull request consolidates color-related tests into packages/flutter/test/material/colors_test.dart, incorporating test suites for HSVColor, HSLColor, ColorSwatch, and color space conversions including Display P3 to extended sRGB. The check_tests_cross_imports.dart configuration is updated to remove the reference to the painting colors test file. Feedback identifies a variable naming inconsistency in the HSVColor saturation sweep test where hslColor is used for an HSVColor instance.

fluttergithubbot · 2026-05-20T02:00:27Z

An existing Git SHA, bbf541009632e33bdda6af333924288c2bba96e2, was detected, and no actions were taken.

To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with --force) that already was pushed before, push a blank commit (git commit --allow-empty -m "Trigger Build") or rebase to continue.

fluttergithubbot · 2026-05-20T02:01:09Z

An existing Git SHA, bbf541009632e33bdda6af333924288c2bba96e2, was detected, and no actions were taken.

To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with --force) that already was pushed before, push a blank commit (git commit --allow-empty -m "Trigger Build") or rebase to continue.

victorsanni

Thanks for this PR. I don't think we want to move painting/colors_test.dart to material/. I looked through the file, only the test 'MaterialColor swatch comparison' is tied to material/, so only that test should move if anything.

In general we do not want to wholesale move tests from the framework into the design libraries unless necessary.

victorsanni · 2026-05-20T21:55:21Z

Converting to draft to remove this PR from our review queue. Feel free to reconvert after addressing comments when this is ready for review.

gemini-code-assist

Code Review

This pull request consolidates and expands color-related tests in packages/flutter/test/material/colors_test.dart, adding comprehensive test suites for HSVColor, HSLColor, ColorSwatch, and color space conversions such as Display P3 to sRGB. It also updates the cross-import checker to reflect these structural changes. Feedback suggests improving the readability of the MaterialColor equality test by using more accurate variable names and adopting more idiomatic assertion syntax.

I am having trouble creating individual review comments. Click here to see my feedback.

packages/flutter/test/material/colors_test.dart (740-756)

The variable name differentMap is misleading because it contains the exact same content as sampleMap. In the context of this test, which verifies that MaterialColor equality correctly handles different map instances with identical content, a name like anotherMap or identicalMap would be more accurate and improve readability. Additionally, using expect(a, b) is more idiomatic in Flutter tests than expect(a == b, true) for testing equality operators.

  test('MaterialColor swatch comparison', () {
    final sampleMap = <int, MaterialColor>{
      0: Colors.lightBlue,
      1: Colors.deepOrange,
      2: Colors.blueGrey,
    };
    final anotherMap = <int, MaterialColor>{
      0: Colors.lightBlue,
      1: Colors.deepOrange,
      2: Colors.blueGrey,
    };
    final first = MaterialColor(0, sampleMap);
    final second = MaterialColor(0, sampleMap);
    final third = MaterialColor(0, anotherMap);
    expect(first, second);
    expect(first, third);
  });

References

Optimize for readability: Code is read more often than it is written. ^(link)

MarlonJD · 2026-05-22T12:18:39Z

Maintainer label request: please add override code freeze. The current code-freeze failure is expected for this test-only move into packages/flutter/test/material/colors_test.dart; after the label lands I will rerun failed/stale checks.

MarlonJD · 2026-05-27T01:03:48Z

This is approved, and the only current status failure appears to be the global tree-status. Could you please add autosubmit when you get a chance?

Piinks · 2026-06-24T22:31:06Z

Given this a cross import change fixing painting importing material/cupertino (versus material/cupertino importing something bad), this is not a critical change for pre-release of material_ui/cupertino_ui.

As such, I've marked this PR as not ready to port to flutter/packages yet.
We'll provide instructions to move this change over to material_ui/cupertino_ui once ready to receive PRs. Thank you!

The changes not in the material or cupertino libraries will remain here, which means in the future this PR will need to be split up.

github-actions · 2026-09-09T12:16:44Z

This pull request contains changes to Material or Cupertino, which are currently frozen in this repository.

Changes should be made in material_ui and/or cupertino_ui in the flutter/packages repository.

Please refer to #188444 for instructions.

gemini-code-assist

Code Review

This pull request moves the MaterialColor swatch comparison test from painting/colors_test.dart to material/colors_test.dart, updates imports in painting/colors_test.dart to avoid importing material.dart, and adds two tests to the cross-import allowlist. Feedback suggests avoiding adding tests to the cross-import allowlist by refactoring their imports or moving them, and recommends updating the MaterialColor swatch comparison test to use standard Color values instead of nested MaterialColor instances.

gemini-code-assist · 2026-09-09T12:18:45Z

+    'packages/flutter/test/painting/decoration_image_lerp_test.dart',
+    'packages/flutter/test/painting/continuous_rectangle_border_test.dart',


Adding new tests to the knownPaintingCrossImports allowlist should be avoided if possible, as the goal is to reduce and eventually eliminate cross-imports from lower-level packages like painting to higher-level packages like material.\n\nPlease check if decoration_image_lerp_test.dart and continuous_rectangle_border_test.dart can be updated to import package:flutter/painting.dart instead of package:flutter/material.dart. If they actually require material-specific features for testing, they should be moved to the test/material/ directory instead of being added to this allowlist.

References

Suggest simplification and refactoring: Assess whether the code can be made simpler or refactored to enhance readability and maintainability. ^(link)

gemini-code-assist · 2026-09-09T12:18:46Z

+  test('MaterialColor swatch comparison', () {
+    final sampleMap = <int, MaterialColor>{
+      0: Colors.lightBlue,
+      1: Colors.deepOrange,
+      2: Colors.blueGrey,
+    };
+    final anotherMap = <int, MaterialColor>{
+      0: Colors.lightBlue,
+      1: Colors.deepOrange,
+      2: Colors.blueGrey,
+    };
+    final first = MaterialColor(0, sampleMap);
+    final second = MaterialColor(0, sampleMap);
+    final third = MaterialColor(0, anotherMap);
+    expect(first, second);
+    expect(first, third);
+  });


Using MaterialColor instances as values inside a MaterialColor's swatch map is unrealistic and can be confusing, as swatches typically map color strengths (e.g., 50, 100) to standard Color instances.\n\nWe can refactor this test to use standard Color values, which makes the test more idiomatic and realistic.

test('MaterialColor swatch comparison', () {\n final sampleMap = <int, Color>{\n 50: const Color(0xFFE1F5FE),\n 100: const Color(0xFFB3E5FC),\n };\n final anotherMap = <int, Color>{\n 50: const Color(0xFFE1F5FE),\n 100: const Color(0xFFB3E5FC),\n };\n final first = MaterialColor(0, sampleMap);\n final second = MaterialColor(0, sampleMap);\n final third = MaterialColor(0, anotherMap);\n expect(first, second);\n expect(first, third);\n });

References

Suggest simplification and refactoring: Assess whether the code can be made simpler or refactored to enhance readability and maintainability. ^(link)

This PR ports the Material-specific test from [flutter/flutter#186777](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](flutter/flutter#188444). The PR that removes the test in the framework is flutter/flutter#192716

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

github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages labels May 20, 2026

MarlonJD mentioned this pull request May 20, 2026

Move Material color tests to material #186774

Closed

gemini-code-assist Bot reviewed May 20, 2026

View reviewed changes

Comment thread packages/flutter/test/material/colors_test.dart Resolved

MarlonJD closed this May 20, 2026

MarlonJD deleted the move-painting-colors-tests-to-material branch May 20, 2026 01:57

MarlonJD restored the move-painting-colors-tests-to-material branch May 20, 2026 01:59

MarlonJD reopened this May 20, 2026

MarlonJD closed this May 20, 2026

MarlonJD deleted the move-painting-colors-tests-to-material branch May 20, 2026 02:00

MarlonJD restored the move-painting-colors-tests-to-material branch May 20, 2026 02:01

MarlonJD reopened this May 20, 2026

Move painting colors tests to material

237a1f0

MarlonJD force-pushed the move-painting-colors-tests-to-material branch from bbf5410 to 237a1f0 Compare May 20, 2026 02:06

victorsanni requested changes May 20, 2026

View reviewed changes

victorsanni marked this pull request as draft May 20, 2026 21:55

Merge branch 'master' into move-painting-colors-tests-to-material

6d66435

MarlonJD marked this pull request as ready for review May 21, 2026 10:33

MarlonJD requested a review from victorsanni May 21, 2026 10:33

Move only Material color comparison test

9f742e6

gemini-code-assist Bot reviewed May 21, 2026

View reviewed changes

MarlonJD and others added 4 commits May 21, 2026 13:41

Use idiomatic MaterialColor equality expectations

d570bed

Merge branch 'master' into move-painting-colors-tests-to-material

78d6995

Merge branch 'master' into move-painting-colors-tests-to-material

b533abc

Merge branch 'master' into move-painting-colors-tests-to-material

e5ace37

victorsanni approved these changes May 21, 2026

View reviewed changes

victorsanni added the CICD Run CI/CD label May 21, 2026

MarlonJD requested a review from victorsanni May 22, 2026 12:22

victorsanni previously approved these changes May 26, 2026

View reviewed changes

Merge branch 'master' into move-painting-colors-tests-to-material

b158c8e

github-actions Bot removed the CICD Run CI/CD label May 27, 2026

victorsanni added CICD Run CI/CD override code freeze Override an active code freeze. and removed CICD Run CI/CD labels May 27, 2026

Merge branch 'master' into move-painting-colors-tests-to-material

0c641cd

github-actions Bot removed the CICD Run CI/CD label May 27, 2026

Piinks removed the override code freeze Override an active code freeze. label Jun 24, 2026

Piinks added Decoupling: Not ready to port yet Instructions will be provided when this is ready to move to flutter/packages. Decoupling: Split PR The PR will need to be split to separate Material & Cupertino changes labels Jun 24, 2026

MarlonJD marked this pull request as draft July 7, 2026 22:06

Merge master into move-painting-colors-tests-to-material

add74a7

MarlonJD dismissed victorsanni’s stale review via add74a7 September 9, 2026 12:16

MarlonJD marked this pull request as ready for review September 9, 2026 12:17

gemini-code-assist Bot reviewed Sep 9, 2026

View reviewed changes

Address review feedback for Material color tests

f2b8fce

MarlonJD mentioned this pull request Sep 13, 2026

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

Merged

MarlonJD closed this Sep 13, 2026

github-project-automation Bot moved this from Todo to Done in Test cross-imports Review Queue Sep 13, 2026

MarlonJD mentioned this pull request Sep 13, 2026

[cross_imports] Remove Material dependency from painting color tests #192716

Open

engine-flutter-autoroll mentioned this pull request Oct 22, 2026

Roll Packages from 138342751909 to 93baed9106f6 (9 revisions) #195068

Merged

		'packages/flutter/test/painting/decoration_image_lerp_test.dart',
		'packages/flutter/test/painting/continuous_rectangle_border_test.dart',

Conversation

MarlonJD commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

fluttergithubbot commented May 20, 2026

Uh oh!

fluttergithubbot commented May 20, 2026

Uh oh!

victorsanni left a comment

Choose a reason for hiding this comment

Uh oh!

victorsanni commented May 20, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

packages/flutter/test/material/colors_test.dart (740-756)

Uh oh!

MarlonJD commented May 22, 2026

Uh oh!

MarlonJD commented May 27, 2026

Uh oh!

Piinks commented Jun 24, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MarlonJD commented May 20, 2026 •

edited

Loading