-
Notifications
You must be signed in to change notification settings - Fork 28.7k
Inject Usage dependency into FallbackDiscovery and BuildEvent #53443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -132,16 +132,14 @@ void main() { | |||
), | |||
); | |||
|
|||
await diagnoseXcodeBuildFailure(buildResult); | |||
await diagnoseXcodeBuildFailure(buildResult, mockUsage, logger); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests still hit the context.
Future<VmService> Function(String wsUri, {Log log}) vmServiceConnectUri = | ||
vm_service_io.vmServiceConnectUri, | ||
}) : _logger = logger, | ||
_mDnsObservatoryDiscovery = mDnsObservatoryDiscovery, | ||
_portForwarder = portForwarder, | ||
_protocolDiscovery = protocolDiscovery, | ||
_flutterUsage = flutterUsage, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_flutterUsage = flutterUsage, | |
_flutterUsage = flutterUsage, |
@@ -48,6 +48,7 @@ void main() { | |||
platform: platform, | |||
processManager: processManager, | |||
terminal: terminal, | |||
usage: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future we'll likely want to make these non-nullable. Consider creating a FlutterUsage.test()
which can be created at low cost to avoid explicitly passing null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other than Jonah's suggestions, LGTM
Oh, and an analyzer error:
|
0b3b425
to
7ca010f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Inject Usage into FallbackDiscovery and BuildEvent.
Convert fallback_discovery_test.dart to testWithoutContext
Related Issues
#47161
Checklist
///
).flutter analyze --flutter-repo
) does not report any problems on my PR.Breaking Change