Skip to content

Conversation

@stuartmorgan-g
Copy link
Collaborator

Updates file_selector_ios, _macos, and _windows tests to use the same structure as file_selector_linux already uses:

  • Removes the use of Pigeon's Dart test generator in favor of DI'ing an implementation of the actual generated Dart API.
  • Removes the use of mockito in favor of a fake, since the fake is extremely simple to maintain and it makes the tests simpler.

Part of flutter/flutter#159886

Pre-Review Checklist

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2 3

@stuartmorgan-g stuartmorgan-g added override: no versioning needed Override the check requiring version bumps for most changes override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Oct 28, 2025
Copy link

@gemini-code-assist gemini-code-assist bot left a 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 refactors the tests for file_selector_ios, file_selector_macos, and file_selector_windows to remove the dependency on Pigeon's Dart test generator and the mockito package. This is achieved by introducing dependency injection for the platform-specific FileSelectorApi and using a simple fake implementation in tests. This change simplifies the test setup, reduces dependencies, and removes generated test files, improving the overall maintainability of the codebase. The changes are consistent across all affected packages and I've noted one opportunity for improvement.

@stuartmorgan-g
Copy link
Collaborator Author

CHANGELOG/version overrides: While technically there is production code change here, it's just adding an optional, visibible-only-for-tests parameter, and moving one ivar initialization from the declaration to the constructor, so it's pretty clearly a no-op.

final bool? selectMultiple;
}

@HostApi(dartHostTestHandler: 'TestFileSelectorApi')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This wasn't being used, it was just copypasta from some other platform, which is why there are no other changes to file_selector_linux.

Copy link
Contributor

@tarrinneal tarrinneal left a comment

Choose a reason for hiding this comment

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

so much cleaner!

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 8, 2025
@auto-submit auto-submit bot merged commit dfa6086 into flutter:main Nov 8, 2025
80 checks passed
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Nov 10, 2025
flutter/packages@3caa48b...4cec230

2025-11-08 [email protected] [google_maps_flutter] Replace
deprecated methods (flutter/packages#10377)
2025-11-08 [email protected] [file_selector] Remove use of
Pigeon's Dart test generator (flutter/packages#10315)
2025-11-07 [email protected] Update Gradle Version Formatting
Validation (flutter/packages#10326)

If this roll has caused a breakage, revert this CL and stop the roller
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
IvoneDjaja pushed a commit to IvoneDjaja/flutter that referenced this pull request Nov 22, 2025
flutter/packages@3caa48b...4cec230

2025-11-08 [email protected] [google_maps_flutter] Replace
deprecated methods (flutter/packages#10377)
2025-11-08 [email protected] [file_selector] Remove use of
Pigeon's Dart test generator (flutter/packages#10315)
2025-11-07 [email protected] Update Gradle Version Formatting
Validation (flutter/packages#10326)

If this roll has caused a breakage, revert this CL and stop the roller
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
mboetger pushed a commit to mboetger/flutter that referenced this pull request Dec 2, 2025
flutter/packages@3caa48b...4cec230

2025-11-08 [email protected] [google_maps_flutter] Replace
deprecated methods (flutter/packages#10377)
2025-11-08 [email protected] [file_selector] Remove use of
Pigeon's Dart test generator (flutter/packages#10315)
2025-11-07 [email protected] Update Gradle Version Formatting
Validation (flutter/packages#10326)

If this roll has caused a breakage, revert this CL and stop the roller
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
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
flutter/packages@3caa48b...4cec230

2025-11-08 [email protected] [google_maps_flutter] Replace
deprecated methods (flutter/packages#10377)
2025-11-08 [email protected] [file_selector] Remove use of
Pigeon's Dart test generator (flutter/packages#10315)
2025-11-07 [email protected] Update Gradle Version Formatting
Validation (flutter/packages#10326)

If this roll has caused a breakage, revert this CL and stop the roller
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

Labels

autosubmit Merge PR when tree becomes green via auto submit App override: no changelog needed Override the check requiring CHANGELOG updates for most changes override: no versioning needed Override the check requiring version bumps for most changes p: file_selector platform-ios platform-linux platform-macos platform-windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants