Skip to content
This repository was archived by the owner on Feb 2, 2021. It is now read-only.

Fix simulator started on run with fake device id. #993

Merged
merged 2 commits into from
Aug 14, 2017
Merged

Fix simulator started on run with fake device id. #993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
70c2339
Fix simulator started on run with fake device id.
Jul 26, 2017
1533d41
Fix comments
Jul 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export class ProvisionType {

export class DeviceTypes {
static Emulator = "Emulator";
static Simulator = "Simulator";
static Device = "Device";
}

Expand Down
2 changes: 1 addition & 1 deletion mobile/android/android-emulator-services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class AndroidEmulatorServices implements Mobile.IAndroidEmulatorServices {
await this.unlockScreen(emulatorId);
} else {
if (emulatorImage) {
this.$errors.fail(this.$messages.Devices.NotFoundDeviceByIdentifierErrorMessageWithIdentifier, emulatorImage, this.$staticConfig.CLIENT_NAME.toLowerCase());
this.$errors.fail(`No emulator image available for device identifier '${emulatorImage}'.`);
} else {
this.$errors.fail(this.$messages.Devices.NotFoundDeviceByIdentifierErrorMessage, this.$staticConfig.CLIENT_NAME.toLowerCase());
}
Expand Down
33 changes: 30 additions & 3 deletions mobile/mobile-core/devices-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,19 +392,34 @@ export class DevicesService extends EventEmitter implements Mobile.IDevicesServi
}
let deviceInstances = this.getDeviceInstances();

//if no --device is passed and no devices are found, the default emulator is started
if (!data.deviceId && _.isEmpty(deviceInstances)) {
if (!this.$hostInfo.isDarwin && this.$mobileHelper.isiOSPlatform(data.platform)) {
this.$errors.failWithoutHelp(constants.ERROR_NO_DEVICES_CANT_USE_IOS_SIMULATOR);
}
}

try {
await this._startEmulatorIfNecessary(data);
} catch (err) {
const errorMessage = this.getEmulatorError(err, data.platform);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the method description, because you changed the method behavior. (now it fails without help)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please help me find the place where to update the description. I can't see where inside the method description is included the way it fails. There are a few places inside the method that also fail without help (that`s the reason I made it fail without help here also).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

failWithoutHelp means that command's help will not be shown, which is fine at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, just describe as best you can what the method does so it's easy to understand.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of getting 10 rows of description what the method does. The name and the code should talk for itself. Some basic info should be enough. But describing that in case A the situation 1 will happen, and in case B situation 2 will happen is not correct, IMO. In case you are interested in all cases - you can always check the implementation.


this.$errors.failWithoutHelp(errorMessage);
}
}
}

private async _startEmulatorIfNecessary(data?: Mobile.IDevicesServicesInitializationOptions): Promise<void> {
const deviceInstances = this.getDeviceInstances();

//if no --device is passed and no devices are found, the default emulator is started
if (!data.deviceId && _.isEmpty(deviceInstances)) {
return await this.startEmulator(data.platform);
}

//check if --device(value) is running, if it's not or it's not the same as is specified, start with name from --device(value)
if (data.deviceId) {
if (!helpers.isNumber(data.deviceId)) {
let activeDeviceInstance = _.find(this.getDeviceInstances(), (device: Mobile.IDevice) => { return device.deviceInfo.identifier === data.deviceId; });
let activeDeviceInstance = _.find(deviceInstances, (device: Mobile.IDevice) => { return device.deviceInfo.identifier === data.deviceId; });
if (!activeDeviceInstance) {
return await this.startEmulator(data.platform, data.deviceId);
}
Expand All @@ -418,7 +433,6 @@ export class DevicesService extends EventEmitter implements Mobile.IDevicesServi
return await this.startEmulator(data.platform);
}
}
}
}

/**
Expand Down Expand Up @@ -672,6 +686,19 @@ export class DevicesService extends EventEmitter implements Mobile.IDevicesServi
shouldReturnImmediateResult = shouldReturnImmediateResult || false;
return { shouldReturnImmediateResult: shouldReturnImmediateResult, platform: platform };
}

private getEmulatorError(error: Error, platform: string): string {
let emulatorName = constants.DeviceTypes.Emulator;

if (this.$mobileHelper.isiOSPlatform(platform)) {
emulatorName = constants.DeviceTypes.Simulator;
}

return `Cannot find connected devices.${EOL}` +
`${emulatorName} start failed with: ${error.message}${EOL}` +
`To list currently connected devices and verify that the specified identifier exists, run '${this.$staticConfig.CLIENT_NAME.toLowerCase()} device'.${EOL}` +
`To list available ${emulatorName.toLowerCase()} images, run '${this.$staticConfig.CLIENT_NAME.toLowerCase()} device <Platform> --available-devices'.`;
}
}

$injector.register("devicesService", DevicesService);