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 CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Jun 25, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 26, 2026
@auto-submit

auto-submit Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/packages/11983, because This PR has not met approval requirements for merging. 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.

@QuncCccccc QuncCccccc 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

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