[flutter_tools] flutter logs no longer requires supported device #66696
Conversation
| Future<List<Device>> pollingGetDevices({ Duration timeout }); | ||
|
|
||
| Future<void> startPolling() async { | ||
| void startPolling() { |
There was a problem hiding this comment.
Most of these methods do not need to be async - this made it much easier to fix the disabled test
| } | ||
| } | ||
|
|
||
| typedef DispatchCommand = void Function(Map<String, dynamic> command); |
There was a problem hiding this comment.
These changes just make the async APIs sync like the underlying code
| @override | ||
| Future<FlutterCommandResult> verifyThenRunCommand(String commandPath) async { | ||
| device = await findTargetDevice(); | ||
| device = await findTargetDevice(includeUnsupportedDevices: true); |
There was a problem hiding this comment.
The only change for the log command
| group('PollingDeviceDiscovery', () { | ||
| testUsingContext('startPolling', () async { | ||
| await FakeAsync().run((FakeAsync time) async { | ||
| testUsingContext('startPolling', () { |
There was a problem hiding this comment.
This test was disabled, now it works again yay
|
@Hixie as the primary user of |
| typedef DispatchCommand = void Function(Map<String, dynamic> command); | ||
|
|
||
| typedef CommandHandler = Future<dynamic> Function(Map<String, dynamic> args); | ||
| typedef CommandHandler = FutureOr<dynamic> Function(Map<String, dynamic> args); |
There was a problem hiding this comment.
FWIW, I would strongly prefer we never use FutureOr. It leads to all kinds of issues later.
There was a problem hiding this comment.
not quite sure i understand where this ends up mattering
There was a problem hiding this comment.
Removed, its not necessary
Hixie
left a comment
There was a problem hiding this comment.
LGTM module the FutureOr. But I suspect that means something else major has to change...?
|
Fixed the FutureOr issue :) , NBD |
|
Landing to kick the tree - the last red test passed but the build status is stuck |
Description
Flutter logs should not attempt to filter the device list based on the current project, because it does not require a current project. Also fix disabled polling test
Fixes #47996
Fixes #63550