refactor: device selection revisited #1694
Conversation
noomorph
force-pushed
the
fix/ios/device-selection
branch
3 times, most recently
from
eedadb7
to
1a99650
Compare
I don't think this is correct. There can be simulators of the same name for different OS versions. We need to be able to specify a more complex query, rather than just a string with commas. |
|
We discussed in the past the ability to provide an object as device selector. vs |
LeoNatan
changed the title
| return udid; | ||
| } | ||
|
|
||
| async _groupDevicesByStatus(query) { |
There was a problem hiding this comment.
Query should be an object rather than string. If we are making such important changes, let's do it fully. Otherwise, there is ambiguity that cannot be resolved with this query. Devices with the same name are allowed to exist, If I have two devices named "MySim", one for iOS 13 and one for iOS 12, the current proposed solution will fail for a "MySim, iOS 13.0" query.
There was a problem hiding this comment.
Perhaps we can have a fallback like this for legacy strings, but we should add the full solution. I don't think it's a hard solution, as the object can travel, just like the string, from the package.json to here.
There was a problem hiding this comment.
I decided to split the refactor and the change to the feature, on Friday it will appear in a consequent pull request. Have to switch to the "performance project". Currently, with this PR the device selection logic will remain the same for iOS simulators, except for removed xcrun simctl list -j.
noomorph
changed the title
noomorph
force-pushed
the
fix/ios/device-selection
branch
from
94472ed
to
8abafca
Compare
fix(ios): create a new device using a busy one as a prototype perf(devices): optimize search and create operations for simulators and emulators refactor(devices): a simpler concept of DeviceRegistry
noomorph
force-pushed
the
fix/ios/device-selection
branch
from
8abafca
to
d4518dc
Compare
This pull request prepares the ground to finally deal with #853 and also should potentially resolve #1565.
The consequent PR will add
deviceproperty to a configuration, which can be configured in a more powerful way.Changes:
AppleSimUtils: make it thinner and more explicit, therefore the search method is called now
list(similar to the CLI). The query handler logic (iPhone X, iOS 12.0) is moved 1 layer upwards, to SimulatorDriver.SimulatorDriver: logic of search remains intact - device type (+ device os, if specified via comma).
AppleSimUtils: remove the dependency on a heavy and non-maintainable direct call
xcrun simctl list -j, by allowingnew AppleSimUtils().create(device)to accept a "prototype" for a device you want to create. I pick it from the result thelist()method returns as-is, and pass it tocreate().SimulatorDriver: remove redundant calls to applesimutils, querying the same things twice when creating a device. Important: now, when creating a device, it takes an OS version of the first matching but a busy device.
AndroidDriver: remove redundant Telnet queries to busy devices. That can happen, because getting a name now done explicitly, via
queryName()method, so we can postpone it till the very last moment.ADB: recompose an async-iterator-like ADB output parser to lazy DeviceHandle classes, to allow bullet (5) to happen.
DeviceRegistry: refactor to accommodate changes to
SimulatorDriverandAndroidDriver. API was simplified to justallocateDeviceanddisposeDevice. This way, we remove 2-way connecting between *Driver -> DeviceRegistry -> injected methods of *Driver, therefore we have better maintainability.TODO(@noomorph): conduct some more manual QA.