-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Inject iOS, Android workflows via context #10750
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
Eliminates the need for the device/daemon code to get at the iOS/Android tooling indirectly via Doctor. In tests, we now inject the workflow objects (or mocks) directly.
| import 'android_sdk.dart'; | ||
| import 'android_studio.dart' as android_studio; | ||
|
|
||
| AndroidWorkflow get androidWorkflow => context[AndroidWorkflow]; |
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.
This will alleviate us from having to aggressively inject it everywhere (like Fuchsia and google-internal)
context.putIfAbsent(AndroidWorkflow, () => new AndroidWorkflow())
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.
Done.
| import '../doctor.dart'; | ||
| import 'mac.dart'; | ||
|
|
||
| IOSWorkflow get iosWorkflow => context[IOSWorkflow]; |
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.
Same here - ... => context.putIfAbsent(...)
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.
Done.
| } | ||
|
|
||
| class MockAndroidWorkflow extends AndroidWorkflow { | ||
| MockAndroidWorkflow({this.canListDevices}); |
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.
default to true
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.
Done.
|
|
||
| class MockIosWorkflow extends IOSWorkflow { | ||
| class MockIOSWorkflow extends IOSWorkflow { | ||
| MockIOSWorkflow({this.canListDevices}); |
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.
And here
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.
Done.
tvolkert
left a comment
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 modulo comments
Eliminates the need for the device/daemon code to get at the iOS/Android tooling indirectly via Doctor. In tests, we now inject the workflow objects (or mocks) directly.
Eliminates the need for the device/daemon code to get at the iOS/Android
tooling indirectly via Doctor. In tests, we now inject the workflow
objects (or mocks) directly.