Skip to content

[material_ui] Port flutter/flutter #186670 "Use local semantics tester in Material selection tests"#11983

Open
MarlonJD wants to merge 3 commits into
flutter:mainfrom
MarlonJD:decoupling-port-186670
Open

[material_ui] Port flutter/flutter #186670 "Use local semantics tester in Material selection tests"#11983
MarlonJD wants to merge 3 commits into
flutter:mainfrom
MarlonJD:decoupling-port-186670

Conversation

@MarlonJD

@MarlonJD MarlonJD commented Jun 25, 2026

Copy link
Copy Markdown

This PR ports flutter/flutter#186670 by @MarlonJD from flutter/flutter to flutter/packages.

It moves the affected Material selection tests from material_ui's temporarily_disabled_tests directory into the main test directory now that their gross ../widgets/semantics_tester.dart import has been fixed to use material_ui's local semantics_tester.dart.

This follows the porting instructions in flutter/flutter#188444:

  • merge commits from the original PR are not included
  • the affected temporarily_disabled_tests files are moved into the main test directory after fixing the gross import

Validation:

  • dart format --set-exit-if-changed packages/material_ui/test/checkbox_test.dart packages/material_ui/test/radio_test.dart packages/material_ui/test/range_slider_test.dart packages/material_ui/test/slider_test.dart packages/material_ui/test/toggle_buttons_test.dart
  • git diff --check
  • flutter test test/checkbox_test.dart test/radio_test.dart test/range_slider_test.dart test/slider_test.dart test/toggle_buttons_test.dart

The targeted test command passed with 304 tests.

@github-actions github-actions Bot added triage-framework Should be looked at in framework triage p: material_ui labels Jun 25, 2026
@MarlonJD MarlonJD force-pushed the decoupling-port-186670 branch from 8c3a514 to 334905e Compare June 25, 2026 12:44
@MarlonJD MarlonJD marked this pull request as ready for review June 25, 2026 12:44

@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 removes the @Skip annotations from several test files, including checkbox_test.dart, radio_test.dart, range_slider_test.dart, slider_test.dart, and toggle_buttons_test.dart. It also updates the import path of semantics_tester.dart from a relative parent directory to a local path, and reformats several test blocks in slider_test.dart. The review feedback points out a grammatical typo in one of the test descriptions in slider_test.dart and suggests correcting 'appear' to 'appears'.

Comment thread packages/material_ui/test/slider_test.dart Outdated
@MarlonJD

Copy link
Copy Markdown
Author

A note on why these files moved out of temporarily_disabled_tests: flutter/flutter#188444 says that when a port fixes a gross import for tests in material_ui/cupertino_ui, those tests may be moved back into the main test directory. These five files were skipped only for the old gross ../widgets/semantics_tester.dart import. This PR fixes that to use material_ui's local semantics_tester.dart, so I moved the affected tests into packages/material_ui/test/ and removed the file-level @Skip.

I also verified the enabled tests locally with:\n\nflutter test test/checkbox_test.dart test/radio_test.dart test/range_slider_test.dart test/slider_test.dart test/toggle_buttons_test.dart

That passed with 304 tests.

@MarlonJD MarlonJD force-pushed the decoupling-port-186670 branch from 334905e to 29324a4 Compare June 25, 2026 12:49
@Piinks Piinks changed the title [Decoupling] Port flutter/flutter #186670 "Use local semantics tester in Material selection tests" [material_ui] Port flutter/flutter #186670 "Use local semantics tester in Material selection tests" Jun 25, 2026

@justinmc justinmc 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 👍

Thanks so much for porting this PR so quickly! Let me know if there's anything in the porting instructions in flutter/flutter#188444 that could be improved.

I see there are still a bunch of formatting changes in this PR, but I'm not sure whether or not they are needed. Let's let CI run and see if it passes, if so then I think it's good.

@justinmc justinmc added the CICD Run CI/CD label Jun 25, 2026
@flutter-dashboard

Copy link
Copy Markdown

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.

@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 25, 2026
@MarlonJD

Copy link
Copy Markdown
Author

Thanks! I pushed ac8073ea4 to address the dashboard merge-conflict blocker. The conflict came from slider_test.dart landing on main first via #11977, so I dropped that file from this port and kept only the remaining four Material selection test moves.

GitHub now reports the PR as mergeable, and tree-status is passing. I also re-ran the local validation for the updated 4-file scope:

  • git diff --check upstream/main...HEAD
  • dart format --set-exit-if-changed packages/material_ui/test/checkbox_test.dart packages/material_ui/test/radio_test.dart packages/material_ui/test/range_slider_test.dart packages/material_ui/test/toggle_buttons_test.dart (0 changed)
  • flutter test test/checkbox_test.dart test/radio_test.dart test/range_slider_test.dart test/toggle_buttons_test.dart (198 tests passed)

One possible improvement for the porting instructions: it may help to include a concrete PR title example, such as [material_ui] Port flutter/flutter #NNNNNN "Original PR title", and to make the temporarily_disabled_tests rule explicit: if a file was skipped only because of the gross ../widgets/semantics_tester.dart import, the port should fix the import, remove the file-level @Skip, and move the file back into packages/<package>/test/. If the equivalent file has already landed on packages/main, leave it out of the port to avoid duplicate move conflicts.

@QuncCccccc

Copy link
Copy Markdown
Contributor

Hi @MarlonJD! Thanks a lot for your contribution! Could you help fix Linux analyze master and stable? Seems the imports need to be sorted.
Screenshot 2026-06-25 at 12 19 50 PM

@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 25, 2026
@MarlonJD

Copy link
Copy Markdown
Author

Fixed in 6c54de724 by sorting the material_ui selection test imports.

Local validation:

  • dart format --set-exit-if-changed packages/material_ui/test/checkbox_test.dart packages/material_ui/test/radio_test.dart packages/material_ui/test/range_slider_test.dart packages/material_ui/test/toggle_buttons_test.dart (0 changed)
  • dart run script/tool/bin/flutter_plugin_tools.dart analyze --packages material_ui --base-branch origin/main --custom-analysis=script/configs/custom_analysis.yaml (No issues found!)
  • flutter test test/checkbox_test.dart test/radio_test.dart test/range_slider_test.dart test/toggle_buttons_test.dart (198 tests passed)

The GitHub label/check-change gates have run; waiting for the remaining Linux analyze master/stable presubmit statuses to be scheduled/reported.

@QuncCccccc QuncCccccc added the CICD Run CI/CD label Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD p: material_ui triage-framework Should be looked at in framework triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants