-
Notifications
You must be signed in to change notification settings - Fork 35
change how we parse simulator output to incorporate try bots #18
Changes from all commits
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 |
|---|---|---|
|
|
@@ -102,7 +102,7 @@ class IosSimulatorManager { | |
| await _listExistingSimulators(osMajorVersion, osMinorVersion); | ||
|
|
||
| // The simulator list, have the version string followed by a list of phone | ||
| // names along with their ids and their statuses. Example output: | ||
| // names along with their ids and their statuses. Example output 1: | ||
| // -- iOS 13.5 -- | ||
| // iPhone 8 (2A437C91-3B85-4D7B-BB91-32561DA07B85) (Shutdown) | ||
| // iPhone 8 Plus (170207A8-7631-4CBE-940E-86A7815AEB2B) (Shutdown) | ||
|
|
@@ -114,11 +114,34 @@ class IosSimulatorManager { | |
| // iPhone 8 Plus (170207A8-7631-4CBE-940E-86A7815AEB2B) (Shutdown) | ||
| // iPhone 11 (7AEC5FB9-E08A-4F7F-8CA2-1518CE3A3E0D) (Booted) | ||
| // iPhone 11 Pro (D8074C8B-35A5-4DA5-9AB2-4CE738A5E5FC) (Shutdown) | ||
| // -- Device Pairs -- | ||
| // Example output 2 (from Mac Web Engine try bots): | ||
| // == Devices == | ||
| // -- iOS 13.0 -- | ||
| // iPhone 8 (C142C9F5-C26E-4EB5-A2B8-915D5BD62FA5) (Shutdown) | ||
| // iPhone 8 Plus (C1FE8FAA-5797-478E-8BEE-D7AD4811F08C) (Shutdown) | ||
| // iPhone 11 (28A3E6C0-76E7-4EE3-9B34-B059C4BBE5CA) (Shutdown) | ||
| // iPhone 11 Pro (0AD4BBA5-7BE7-415D-B9FD-D962FA8E1782) (Shutdown) | ||
| // iPhone 11 Pro Max (1280DE05-B334-4E60-956F-4A62220DEFA3) (Shutdown) | ||
| // iPad Pro (9.7-inch) (EDE46501-CB2B-4EA4-8B5C-13FAC6F2EC91) (Shutdown) | ||
| // iPad Pro (11-inch) (E0B89C9C-6200-495C-B18B-0078CCAAC688) (Shutdown) | ||
| // iPad Pro (12.9-inch) (3rd generation) (DB3EB7A8-C4D2-4F86-AFC1-D652FB0579E8) (Shutdown) | ||
| // iPad Air (3rd generation) (9237DCD8-8F0E-40A6-96DF-B33C915AFE1B) (Shutdown) | ||
| // == Device Pairs == | ||
|
Contributor
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. I think this implementation can be simplified if it first split the output line by line, then inspected each line. There are only two interesting line formats:
A single line-by-line
Contributor
Author
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. thanks a lot for the suggestion, I think I'll used --json tag and will get rid of this parsing all together :) |
||
| final int indexOfVersionListStart = | ||
| simulatorsList.indexOf(simulatorVersion); | ||
| final String restOfTheOutput = simulatorsList | ||
| .substring(indexOfVersionListStart + simulatorVersion.length); | ||
|
Contributor
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. What if
Contributor
Author
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. This will also fail an exception like now. This code relies on the current formatting very much: "--" or "==" kills all the code. I think @christopherfujino suggestion would be the best change. |
||
| final int indexOfNextVersion = restOfTheOutput.indexOf('--'); | ||
| int indexOfNextVersion = restOfTheOutput.indexOf('--'); | ||
|
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. can we do a more specific regex, rather than relying on 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. Hmm, never mind...I see that would be difficult... |
||
| if (indexOfNextVersion == -1) { | ||
| // Search for `== Device Pairs ==`. | ||
| indexOfNextVersion = restOfTheOutput.indexOf('=='); | ||
| } | ||
| if (indexOfNextVersion == -1) { | ||
| // Set to end of file. | ||
| indexOfNextVersion = restOfTheOutput.length; | ||
| } | ||
|
|
||
| final String listOfPhones = | ||
| restOfTheOutput.substring(0, indexOfNextVersion); | ||
|
|
||
|
|
@@ -198,8 +221,7 @@ class IosSimulator { | |
| await io.Process.run('xcrun', ['simctl', 'shutdown', '$id']); | ||
|
|
||
| if (versionResult.exitCode != 0) { | ||
| throw Exception( | ||
| 'Failed to shutdown iOS simulators with id: $id.'); | ||
| throw Exception('Failed to shutdown iOS simulators with id: $id.'); | ||
| } | ||
|
|
||
| this._booted = false; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| // @dart = 2.6 | ||
| import 'package:test/test.dart'; | ||
|
|
||
| import '../lib/simulator_manager.dart'; | ||
|
|
||
| void main() async { | ||
| test('boot simulator', () async { | ||
| IosSimulatorManager simulatorManager = IosSimulatorManager(); | ||
| IosSimulator simulator = | ||
| await simulatorManager.getSimulator(13, 0, 'iPhone 11'); | ||
| await simulator.boot(); | ||
| }); | ||
|
|
||
| test('shutdown simulator', () async { | ||
| IosSimulatorManager simulatorManager = IosSimulatorManager(); | ||
| IosSimulator simulator = | ||
| await simulatorManager.getSimulator(13, 0, 'iPhone 11'); | ||
| await simulator.shutdown(); | ||
| }); | ||
| } |
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.
Is this example demonstrating the need for the new code? If so, then I'm not sure how it does that.
restOfTheOutput.indexOf('--')is >0 (because the string does contain--), so the new code won't execute 🤔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 one of them the version was finishing with '--' now it finishes with "==". let me write an explanation, but as mentioned in the earlier comment my next PR would be just removing these.
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.
I have local test for these things btw which I don't merge let me also send it!