Sitelet https://github.com/flutter/flutter/pull/190768
Skip to content

[tool] Migrate DevicesCommand to modular dependency injection - #190768

Merged
auto-submit[bot] merged 11 commits into
flutter:masterfrom
bkonyi:di/09-devices
Sep 4, 2026
Merged

[tool] Migrate DevicesCommand to modular dependency injection#190768
auto-submit[bot] merged 11 commits into
flutter:masterfrom
bkonyi:di/09-devices

Conversation

@bkonyi

@bkonyi bkonyi commented Aug 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Part 10 of the modular dependency injection migration.

  • Migrates DevicesCommand and DevicesCommandOutput to constructor dependency injection with zero global fallbacks: 
    DevicesCommand({
  required DeviceManager deviceManager,
  required Doctor doctor,
  required super.toolContext,
  super.verboseHelp,
})
  • Requires non-nullable DeviceManager, Doctor, and ToolContext, eliminating ambient globals.* fallbacks and imports from devices.dart.
  • Destructures toolContext.logger and toolContext.terminal for CLI device output and deprecation warnings.
  • Wires dependencies via toolDependencies.toolContext in executable.dart.
  • Migrates unit tests in packages/flutter_tools/test/commands.shard/hermetic/devices_test.dart to hermetic testWithoutContext using FakeToolContext, while preserving permeable suite tests in packages/flutter_tools/test/commands.shard/permeable/devices_test.dart.

Part of #188471

@github-actions github-actions Bot added tool Affects the "flutter" command-line tool. See also t: labels. team-android Owned by Android platform team team-ios Owned by iOS platform team team-macos Owned by the macOS platform team labels Aug 8, 2026
@bkonyi
bkonyi force-pushed the di/09-devices branch 6 times, most recently from a525b61 to 129c560 Compare August 12, 2026 18:17
@github-actions github-actions Bot removed the team-android Owned by Android platform team label Aug 12, 2026
@github-actions github-actions Bot removed team-ios Owned by iOS platform team team-macos Owned by the macOS platform team labels Aug 28, 2026
@bkonyi
bkonyi marked this pull request as ready for review September 2, 2026 12:44
@bkonyi bkonyi added the CICD Run CI/CD label Sep 2, 2026
@flutter-dashboard

flutter-dashboard Bot commented Sep 2, 2026

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.

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 2, 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 refactors DevicesCommand to support dependency injection for DeviceManager, Doctor, and toolContext (including Logger, Platform, and AnsiTerminal), reducing its reliance on global variables. Consequently, the associated hermetic and permeable tests are updated to use testWithoutContext and explicit fakes instead of context-based overrides. There are no review comments, and I have no feedback to provide.

@bkonyi

bkonyi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 2, 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 refactors DevicesCommand to use dependency injection for ToolContext, DeviceManager, and Doctor instead of relying on global variables, and updates the corresponding tests to use testWithoutContext with fake dependencies. Feedback suggests making deviceManager and doctor required parameters in the DevicesCommand constructor to prevent potential silent runtime failures when they are omitted.

Comment thread packages/flutter_tools/lib/src/commands/devices.dart Outdated
@bkonyi

bkonyi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 2, 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 refactors DevicesCommand to accept DeviceManager, Doctor, and ToolContext as constructor arguments instead of accessing them via global variables, enabling the migration of several tests from testUsingContext to testWithoutContext. The review feedback identifies that the required doctor parameter is missing from the DevicesCommand constructor calls in devices_test.dart under the permeable shard, which will lead to compilation failures.

Comment thread packages/flutter_tools/test/commands.shard/permeable/devices_test.dart
Comment thread packages/flutter_tools/test/commands.shard/permeable/devices_test.dart
@bkonyi

bkonyi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 2, 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 refactors DevicesCommand to accept DeviceManager, Doctor, and ToolContext as constructor parameters instead of relying on ambient globals, allowing associated tests to be migrated from testUsingContext to testWithoutContext. The review feedback notes that making these parameters required breaks backwards compatibility with external callers, and suggests making them optional with fallbacks to globals. Additionally, the feedback points out several redundant null checks on _deviceManager that can be removed now that it is non-nullable.

Comment thread packages/flutter_tools/lib/src/commands/devices.dart Outdated
Comment thread packages/flutter_tools/lib/src/commands/devices.dart Outdated
Comment thread packages/flutter_tools/lib/src/commands/devices.dart Outdated
Comment thread packages/flutter_tools/lib/src/commands/devices.dart Outdated
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 2, 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 refactors DevicesCommand and DevicesCommandOutput to support dependency injection of DeviceManager, Doctor, and toolContext, enabling the migration of tests from testUsingContext to testWithoutContext using fakes. Feedback on the changes highlights a potential runtime crash due to a force-unwrap on toolContext when it is omitted for backwards compatibility, and a compile-time error caused by marking optional parameters as required in the DevicesCommandOutput._private constructor.

Comment thread packages/flutter_tools/lib/src/commands/devices.dart Outdated
Comment thread packages/flutter_tools/lib/src/commands/devices.dart
Comment thread packages/flutter_tools/lib/src/commands/devices.dart
Comment thread packages/flutter_tools/lib/src/commands/devices.dart
@bkonyi

bkonyi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 2, 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 refactors DevicesCommand and DevicesCommandOutput to accept injected dependencies for DeviceManager, Doctor, and ToolContext, reducing reliance on global variables. Consequently, associated tests are migrated from testUsingContext to testWithoutContext using fake implementations. The feedback suggests making the deviceManager parameter required in the DevicesCommandOutput factory constructor to completely eliminate global fallback references.

Comment thread packages/flutter_tools/lib/src/commands/devices.dart
@bkonyi

bkonyi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 2, 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 refactors DevicesCommand and DevicesCommandOutput to support dependency injection of DeviceManager, Doctor, and toolContext, enabling the migration of tests from zone-based context overrides to hermetic unit tests. The review feedback suggests simplifying the instantiation of DevicesCommand by removing redundant arguments, removing unnecessary super. prefixes when accessing toolContext, adding documentation to the public DevicesCommandOutput class, and reordering parameters in the private constructor of DevicesCommandOutput for consistency.

Comment thread packages/flutter_tools/lib/executable.dart
Comment thread packages/flutter_tools/lib/src/commands/devices.dart Outdated
Comment thread packages/flutter_tools/lib/src/commands/devices.dart Outdated
Comment thread packages/flutter_tools/lib/src/commands/devices.dart
Comment thread packages/flutter_tools/lib/src/commands/devices.dart
@bkonyi

bkonyi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 2, 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 refactors DevicesCommand and DevicesCommandOutput to accept dependencies like DeviceManager, Doctor, and toolContext via their constructors instead of relying on global variables. This change enables the migration of several tests from testUsingContext to testWithoutContext by passing fake dependencies directly. Feedback suggests refactoring the helper methods _getAttachedDevices and _getWirelessDevices to use the now non-nullable _deviceManager field directly, which would remove redundant parameters.

Comment thread packages/flutter_tools/lib/src/commands/devices.dart Outdated
@bkonyi

bkonyi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 2, 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 refactors DevicesCommand and DevicesCommandOutput to support dependency injection of DeviceManager, Doctor, and toolContext instead of relying on global variables. Consequently, hermetic and permeable tests are updated to use testWithoutContext by passing fake dependencies directly. Feedback suggests simplifying the resolution of the terminal variable in DevicesCommand.validateCommand by falling back to logger.terminal instead of globals.logger.terminal.

Comment thread packages/flutter_tools/lib/src/commands/devices.dart Outdated
@bkonyi

bkonyi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@bkonyi

bkonyi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 2, 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 refactors DevicesCommand and DevicesCommandOutput to use dependency injection for DeviceManager, Doctor, and toolContext instead of relying on global variables. This change allows the associated tests to be refactored from testUsingContext to testWithoutContext. Feedback was provided to simplify the fallback logic for the terminal variable by using the resolved logger instance.

Comment thread packages/flutter_tools/lib/src/commands/devices.dart
@gemini-code-assist

gemini-code-assist Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@bkonyi
bkonyi requested a review from chingjun September 3, 2026 19:33
@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 4, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Sep 4, 2026
Merged via the queue into flutter:master with commit 06071c4 Sep 4, 2026
24 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 4, 2026
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 5, 2026
Merged
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Sep 5, 2026
@engine-flutter-autoroll
Roll Flutter from 5a6cfa7f3d26 to 63170e984e19 (15 revisions) (#12767)
2c91ffe 
flutter/flutter@5a6cfa7...63170e9

2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 6882c3b4b542 to 77094991d1c1 (1 revision) (flutter/flutter#192339)
2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 10652f9d64d6 to db200e27a634 (1 revision) (flutter/flutter#192338)
2026-09-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from FgDQeF6jb1dVRVh3K... to _IgixhH4vEgdz9Oqn... (flutter/flutter#192333)
2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 5744c2480a12 to 6882c3b4b542 (1 revision) (flutter/flutter#192331)
2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 9b1e5fd08d2c to 10652f9d64d6 (4 revisions) (flutter/flutter#192329)
2026-09-05 codefu@google.com ci(engine): target ignore_phone|none for new macs (flutter/flutter#192317)
2026-09-04 okorohelijah@google.com [devicelab] Fix Mac ios_universal_link_test CI build and scheme configuration (flutter/flutter#192321)
2026-09-04 engine-flutter-autoroll@skia.org Roll Dart SDK from 5501d02b583d to 5744c2480a12 (5 revisions) (flutter/flutter#192316)
2026-09-04 okorohelijah@google.com [iOS] Add native deep link lifecycle integration tests for UIScene plugins (flutter/flutter#192173)
2026-09-04 engine-flutter-autoroll@skia.org Roll Skia from 93ac1e630d1d to 9b1e5fd08d2c (3 revisions) (flutter/flutter#192314)
2026-09-04 bkonyi@google.com [tool] Migrate AssembleCommand and GenerateCommand to modular dependency injection (flutter/flutter#190773)
2026-09-04 bkonyi@google.com [tool] Migrate Apple build subcommands and toolchain to modular dependency injection (flutter/flutter#190780)
2026-09-04 bkonyi@google.com [flutter_tools] Safely handle non-JSON messages in test stream parsers (flutter/flutter#192089)
2026-09-04 bkonyi@google.com [tool] Migrate LogsCommand to modular dependency injection (flutter/flutter#190765)
2026-09-04 bkonyi@google.com [tool] Migrate DevicesCommand to modular dependency injection (flutter/flutter#190768)

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
Please CC stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: 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
victorsanni pushed a commit to victorsanni/packages that referenced this pull request Sep 9, 2026
@engine-flutter-autoroll @victorsanni
Roll Flutter from 5a6cfa7f3d26 to 63170e984e19 (15 revisions) (flutte…
6eb90c9 
…r#12767)

flutter/flutter@5a6cfa7...63170e9

2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 6882c3b4b542 to 77094991d1c1 (1 revision) (flutter/flutter#192339)
2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 10652f9d64d6 to db200e27a634 (1 revision) (flutter/flutter#192338)
2026-09-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from FgDQeF6jb1dVRVh3K... to _IgixhH4vEgdz9Oqn... (flutter/flutter#192333)
2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 5744c2480a12 to 6882c3b4b542 (1 revision) (flutter/flutter#192331)
2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 9b1e5fd08d2c to 10652f9d64d6 (4 revisions) (flutter/flutter#192329)
2026-09-05 codefu@google.com ci(engine): target ignore_phone|none for new macs (flutter/flutter#192317)
2026-09-04 okorohelijah@google.com [devicelab] Fix Mac ios_universal_link_test CI build and scheme configuration (flutter/flutter#192321)
2026-09-04 engine-flutter-autoroll@skia.org Roll Dart SDK from 5501d02b583d to 5744c2480a12 (5 revisions) (flutter/flutter#192316)
2026-09-04 okorohelijah@google.com [iOS] Add native deep link lifecycle integration tests for UIScene plugins (flutter/flutter#192173)
2026-09-04 engine-flutter-autoroll@skia.org Roll Skia from 93ac1e630d1d to 9b1e5fd08d2c (3 revisions) (flutter/flutter#192314)
2026-09-04 bkonyi@google.com [tool] Migrate AssembleCommand and GenerateCommand to modular dependency injection (flutter/flutter#190773)
2026-09-04 bkonyi@google.com [tool] Migrate Apple build subcommands and toolchain to modular dependency injection (flutter/flutter#190780)
2026-09-04 bkonyi@google.com [flutter_tools] Safely handle non-JSON messages in test stream parsers (flutter/flutter#192089)
2026-09-04 bkonyi@google.com [tool] Migrate LogsCommand to modular dependency injection (flutter/flutter#190765)
2026-09-04 bkonyi@google.com [tool] Migrate DevicesCommand to modular dependency injection (flutter/flutter#190768)

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
Please CC stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: 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
victorsanni pushed a commit to victorsanni/packages that referenced this pull request Sep 9, 2026
@engine-flutter-autoroll @victorsanni
Roll Flutter from 5a6cfa7f3d26 to 63170e984e19 (15 revisions) (flutte…
bfa5aed 
…r#12767)

flutter/flutter@5a6cfa7...63170e9

2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 6882c3b4b542 to 77094991d1c1 (1 revision) (flutter/flutter#192339)
2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 10652f9d64d6 to db200e27a634 (1 revision) (flutter/flutter#192338)
2026-09-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from FgDQeF6jb1dVRVh3K... to _IgixhH4vEgdz9Oqn... (flutter/flutter#192333)
2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 5744c2480a12 to 6882c3b4b542 (1 revision) (flutter/flutter#192331)
2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 9b1e5fd08d2c to 10652f9d64d6 (4 revisions) (flutter/flutter#192329)
2026-09-05 codefu@google.com ci(engine): target ignore_phone|none for new macs (flutter/flutter#192317)
2026-09-04 okorohelijah@google.com [devicelab] Fix Mac ios_universal_link_test CI build and scheme configuration (flutter/flutter#192321)
2026-09-04 engine-flutter-autoroll@skia.org Roll Dart SDK from 5501d02b583d to 5744c2480a12 (5 revisions) (flutter/flutter#192316)
2026-09-04 okorohelijah@google.com [iOS] Add native deep link lifecycle integration tests for UIScene plugins (flutter/flutter#192173)
2026-09-04 engine-flutter-autoroll@skia.org Roll Skia from 93ac1e630d1d to 9b1e5fd08d2c (3 revisions) (flutter/flutter#192314)
2026-09-04 bkonyi@google.com [tool] Migrate AssembleCommand and GenerateCommand to modular dependency injection (flutter/flutter#190773)
2026-09-04 bkonyi@google.com [tool] Migrate Apple build subcommands and toolchain to modular dependency injection (flutter/flutter#190780)
2026-09-04 bkonyi@google.com [flutter_tools] Safely handle non-JSON messages in test stream parsers (flutter/flutter#192089)
2026-09-04 bkonyi@google.com [tool] Migrate LogsCommand to modular dependency injection (flutter/flutter#190765)
2026-09-04 bkonyi@google.com [tool] Migrate DevicesCommand to modular dependency injection (flutter/flutter#190768)

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
Please CC stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: 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

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

CICD Run CI/CD tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bkonyi @chingjun