-
Notifications
You must be signed in to change notification settings - Fork 26.7k
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
Prevent crashes on range errors when selecting device #129290
Changes from 6 commits
b6b6094
472c8fe
5326296
931565d
530f8a9
dc1315a
2091fcd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1459,7 +1459,7 @@ No devices found yet. Checking for wireless devices... | |
logger: logger, | ||
); | ||
targetDevices.waitForWirelessBeforeInput = true; | ||
targetDevices.deviceSelection.input = '1'; | ||
targetDevices.deviceSelection.input = <String>['1']; | ||
logger.originalStatusText = ''' | ||
Connected devices: | ||
target-device-9 (mobile) • xxx • ios • iOS 16 | ||
|
@@ -1486,6 +1486,44 @@ Please choose one (or "q" to quit): ''')); | |
}, overrides: <Type, Generator>{ | ||
AnsiTerminal: () => terminal, | ||
}); | ||
|
||
|
||
testUsingContext('handle invalid options for device', () async { | ||
deviceManager.iosDiscoverer.deviceList = <Device>[nonEphemeralDevice]; | ||
|
||
final TestTargetDevicesWithExtendedWirelessDeviceDiscovery targetDevices = TestTargetDevicesWithExtendedWirelessDeviceDiscovery( | ||
deviceManager: deviceManager, | ||
logger: logger, | ||
); | ||
targetDevices.waitForWirelessBeforeInput = true; | ||
|
||
// Having the `0` first is an invalid choice for a device, the second | ||
// item in the list, `1` is a valid option though which will return a | ||
// valid device | ||
targetDevices.deviceSelection.input = <String>['0', '1']; | ||
logger.originalStatusText = ''' | ||
Connected devices: | ||
target-device-9 (mobile) • xxx • ios • iOS 16 | ||
|
||
Checking for wireless devices... | ||
|
||
[1]: target-device-9 (xxx) | ||
'''; | ||
|
||
final List<Device>? devices = await targetDevices.findAllTargetDevices(); | ||
|
||
expect(logger.statusText, equals(''' | ||
Connected devices: | ||
target-device-9 (mobile) • xxx • ios • iOS 16 | ||
|
||
No wireless devices were found. | ||
|
||
[1]: target-device-9 (xxx) | ||
Please choose one (or "q" to quit): ''')); | ||
expect(devices, <Device>[nonEphemeralDevice]); | ||
}, overrides: <Type, Generator>{ | ||
AnsiTerminal: () => terminal, | ||
}); | ||
}); | ||
|
||
group('without stdinHasTerminal', () { | ||
|
@@ -1753,7 +1791,7 @@ Checking for wireless devices... | |
]; | ||
|
||
targetDevices.waitForWirelessBeforeInput = true; | ||
targetDevices.deviceSelection.input = '3'; | ||
targetDevices.deviceSelection.input = <String>['3']; | ||
logger.originalStatusText = ''' | ||
Connected devices: | ||
target-device-1 (mobile) • xxx • ios • iOS 16 | ||
|
@@ -1791,7 +1829,7 @@ Please choose one (or "q" to quit): ''')); | |
deviceManager.iosDiscoverer.deviceList = <Device>[attachedIOSDevice1, attachedIOSDevice2]; | ||
|
||
targetDevices.waitForWirelessBeforeInput = true; | ||
targetDevices.deviceSelection.input = '2'; | ||
targetDevices.deviceSelection.input = <String>['2']; | ||
logger.originalStatusText = ''' | ||
Connected devices: | ||
target-device-1 (mobile) • xxx • ios • iOS 16 | ||
|
@@ -1828,7 +1866,7 @@ Please choose one (or "q" to quit): ''')); | |
deviceManager.iosDiscoverer.refreshDeviceList = <Device>[connectedWirelessIOSDevice1, connectedWirelessIOSDevice2]; | ||
|
||
targetDevices.waitForWirelessBeforeInput = true; | ||
targetDevices.deviceSelection.input = '2'; | ||
targetDevices.deviceSelection.input = <String>['2']; | ||
terminal.setPrompt(<String>['1', '2', 'q', 'Q'], '1'); | ||
|
||
final List<Device>? devices = await targetDevices.findAllTargetDevices(); | ||
|
@@ -1906,7 +1944,7 @@ target-device-5 (mobile) • xxx • ios • iOS 16 | |
deviceManager.iosDiscoverer.deviceList = <Device>[attachedIOSDevice1, attachedIOSDevice2]; | ||
|
||
targetDevices.waitForWirelessBeforeInput = true; | ||
targetDevices.deviceSelection.input = '2'; | ||
targetDevices.deviceSelection.input = <String>['2']; | ||
logger.originalStatusText = ''' | ||
Connected devices: | ||
target-device-1 (mobile) • xxx • ios • iOS 16 | ||
|
@@ -1951,7 +1989,7 @@ Please choose one (or "q" to quit): ''')); | |
deviceManager.iosDiscoverer.refreshDeviceList = <Device>[attachedIOSDevice1, attachedIOSDevice2, connectedWirelessIOSDevice1]; | ||
|
||
targetDevices.waitForWirelessBeforeInput = true; | ||
targetDevices.deviceSelection.input = '2'; | ||
targetDevices.deviceSelection.input = <String>['2']; | ||
logger.originalStatusText = ''' | ||
Connected devices: | ||
target-device-1 (mobile) • xxx • ios • iOS 16 | ||
|
@@ -2133,7 +2171,7 @@ target-device-6 (mobile) • xxx • ios • iOS 16 | |
]; | ||
|
||
targetDevices.waitForWirelessBeforeInput = true; | ||
targetDevices.deviceSelection.input = '3'; | ||
targetDevices.deviceSelection.input = <String>['3']; | ||
logger.originalStatusText = ''' | ||
Found multiple devices with name or id matching target-device: | ||
target-device-1 (mobile) • xxx • ios • iOS 16 | ||
|
@@ -2173,7 +2211,7 @@ Please choose one (or "q" to quit): ''')); | |
deviceManager.iosDiscoverer.deviceList = <Device>[attachedIOSDevice1, attachedIOSDevice2]; | ||
|
||
targetDevices.waitForWirelessBeforeInput = true; | ||
targetDevices.deviceSelection.input = '2'; | ||
targetDevices.deviceSelection.input = <String>['2']; | ||
logger.originalStatusText = ''' | ||
Found multiple devices with name or id matching target-device: | ||
target-device-1 (mobile) • xxx • ios • iOS 16 | ||
|
@@ -2443,11 +2481,21 @@ class TestTargetDevicesWithExtendedWirelessDeviceDiscovery extends TargetDevices | |
class TestTargetDeviceSelection extends TargetDeviceSelection { | ||
TestTargetDeviceSelection(super.logger); | ||
|
||
String input = ''; | ||
List<String> input = <String>[]; | ||
|
||
@override | ||
Future<String> readUserInput() async { | ||
return input; | ||
// If only one value is provided for the input, continue | ||
// to return that one input value without popping | ||
// | ||
// If more than one input values are provided, we are simulating | ||
// the user selecting more than one option for a device, so we will pop | ||
// them out from the front | ||
if (input.length > 1) { | ||
return input.removeAt(0); | ||
} | ||
|
||
return input[0]; | ||
Comment on lines
+2489
to
+2503
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hey @vashworth, i reworked this method so that we provide lists of inputs for selecting a device. Thoughts? The while loop in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense to me! |
||
} | ||
} | ||
|
||
|
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.
Could you add another input here that's out of range on the top end too (so 2 in this case)?
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.
Oh good catch!