Repository navigation
Conversation
… and fix macOS test platform
…ework build commands
…ff-Refiner-self-adb31f30
Migrates `_toolContext` and `_appleContext` field extractions to Dart 3 object pattern destructuring in `BuildIOSArchiveCommand.runCommand`.
Migrates context field extractions to Dart 3 object pattern destructuring in `build_ios_framework.dart` and `build_macos_framework.dart`.
…lar context objects
Refactors XcodeCodeSigningSettings to provide context constructors
(XcodeCodeSigningSettings({required PlistParser plistParser, required ToolContext toolContext})
and XcodeCodeSigningSettings.fromContexts({required AppleContext appleContext, required ToolContext toolContext}))
instead of requiring callers to unpack 8+ individual arguments. Adds
DarwinAddToAppCodesigning.fromContexts and refactors BuildCommand to instantiate
codesigning from context objects and pass directly to subcommands.
…dependency injection Migrates BuildIOSCommand and BuildIOSArchiveCommand in build_ios.dart to receive AppleContext, BuildSystem, and ToolContext via constructor dependency injection, eliminating ambient globals reads. Updates build.dart to pass the injected contexts and uses XcodeCodeSigningSettings.fromContexts.
Migrates BuildMacosCommand in build_macos.dart to receive BuildSystem and ToolContext via constructor dependency injection, eliminating ambient globals reads. Updates build.dart to pass the injected contexts.
…d to modular dependency injection Migrates BuildIOSFrameworkCommand and BuildMacOSFrameworkCommand to receive AppleContext and ToolContext via constructor dependency injection, eliminating ambient globals reads. Updates build.dart to pass the injected contexts.
…ipa tests Remove ambient `context.get<*>` fallbacks from `createBuildCommand` in both `build_ios_test.dart` and `build_ipa_test.dart`, defaulting directly to local test doubles. Bridge `ToolContext` and `AppleContext` dependencies into `contextOverrides` in `TestFlutterCommandRunner` during `runCommand` so child and unmigrated downstream commands resolve fakes without test-level `overrides`.
Converts `build_darwin_framework_test.dart` from `testUsingContext` to `testWithoutContext`:
- Avoids ambient `context.get<*>` fallbacks in test factories, routing test doubles directly through `FakeToolContext` and `FakeAppleContext`.
- Supplies `Artifacts.test()` to `FakeToolContext` for native asset manifest checks.
- Guards `processPodsIfNeeded` under `if (boolArg('plugins'))` in `BuildIOSFrameworkCommand` and `BuildMacOSFrameworkCommand`.
- Short-circuits Swift Package Manager checks in `processPodsIfNeeded` when `forceCocoaPodsOnly` is set.
- Bridges `ToolContext` and `AppleContext` test doubles into `TestFlutterCommandRunner` context overrides.
…rrides Ensure that topLevelLogger is provided in contextOverrides even when verbose is false, so commands using ambient globals.printStatus route output to the runner's logger during tests.
There was a problem hiding this comment.
Code Review
This pull request refactors several build commands, including BuildIOSCommand, BuildIOSFrameworkCommand, and BuildMacOSFrameworkCommand, to accept and use AppleContext and ToolContext directly instead of relying on global variables, which allows tests to run hermetically without global context overrides. The review feedback suggests using Dart 3 pattern destructuring to extract dependencies from toolContext consistently across commands, destructuring logger in BuildIOSArchiveCommand.runCommand, and adding the required documentation to the public appleContext getter in BuildFrameworkCommand to comply with the Repository Style Guide.
| final ToolContext( | ||
| :FileSystem fs, | ||
| :OperatingSystemUtils os, | ||
| :ProcessUtils processUtils, | ||
| :Terminal terminal, | ||
| ) = _toolContext; |
There was a problem hiding this comment.
The logger variable is used extensively in runCommand but is not destructured from _toolContext like other dependencies. To ensure consistency and avoid relying on implicit getters or global fallbacks, please destructure logger from _toolContext.
| final ToolContext( | |
| :FileSystem fs, | |
| :OperatingSystemUtils os, | |
| :ProcessUtils processUtils, | |
| :Terminal terminal, | |
| ) = _toolContext; | |
| final ToolContext( | |
| :FileSystem fs, | |
| :Logger logger, | |
| :OperatingSystemUtils os, | |
| :ProcessUtils processUtils, | |
| :Terminal terminal, | |
| ) = _toolContext; |
| final FileSystem fs = toolContext.fs; | ||
| final Logger logger = toolContext.logger; | ||
| final ProcessManager processManager = toolContext.processManager; |
There was a problem hiding this comment.
For consistency with other commands in this PR, consider using Dart 3 pattern destructuring to extract dependencies from toolContext instead of individual variable assignments.
| final FileSystem fs = toolContext.fs; | |
| final Logger logger = toolContext.logger; | |
| final ProcessManager processManager = toolContext.processManager; | |
| final ToolContext( | |
| :FileSystem fs, | |
| :Logger logger, | |
| :ProcessManager processManager, | |
| ) = toolContext; |
| final BuildSystem _buildSystem; | ||
| final ToolContext _toolContext; | ||
|
|
||
| AppleContext get appleContext => _appleContext; |
There was a problem hiding this comment.
The public getter appleContext is missing documentation. According to the Repository Style Guide (line 62), all public members should have documentation.
| AppleContext get appleContext => _appleContext; | |
| /// The Apple-specific context dependencies. | |
| AppleContext get appleContext => _appleContext; |
References
- All public members should have documentation. (link)
|
|
||
| @override | ||
| Future<FlutterCommandResult> runCommand() async { | ||
| final ProcessManager processManager = toolContext.processManager; |
There was a problem hiding this comment.
For consistency with other commands in this PR, consider using Dart 3 pattern destructuring to extract dependencies from toolContext instead of individual variable assignments.
| final ProcessManager processManager = toolContext.processManager; | |
| final ToolContext( | |
| :FileSystem fs, | |
| :Logger logger, | |
| :ProcessManager processManager, | |
| ) = toolContext; |
…ramework-commands
…cos-framework-command
…cos-framework-command
…cos-framework-command
|
Closing as consolidated into #192252. |
…ion (flutter#192252) ## Summary Part 15d of the modular dependency injection migration. Migrates Darwin framework build commands (`BuildIOSFrameworkCommand` and `BuildMacOSFrameworkCommand`) to modular dependency injection with `AppleContext` and `ToolContext`, requiring non-null context parameters and eliminating direct dependencies and fallbacks on `globals.dart` and AppContext. Consolidates PR flutter#192456 into this PR. Targeting `master` directly. Part of flutter#47161
Summary
Part 15e of the modular dependency injection migration.
Migrates
BuildMacOSFrameworkCommand(flutter build macos-framework) to modular dependency injection withAppleContextandToolContext, eliminating direct dependencies onglobals.dart.Stacked on #192252
Compare: bkonyi/flutter@di/15d-darwin-framework-commands...di/15e-macos-framework-command
Part of #47161