Skip to content

Commit caa9c99

Browse files
author
Dimitar Kerezov
committed
Fix PR Comments
1 parent ab41a25 commit caa9c99

File tree

7 files changed

+40
-28
lines changed

7 files changed

+40
-28
lines changed

lib/common

lib/constants.ts

+1
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ export const POST_INSTALL_COMMAND_NAME = "post-install-cli";
9494
export class DebugCommandErrors {
9595
public static UNABLE_TO_USE_FOR_DEVICE_AND_EMULATOR = "The options --for-device and --emulator cannot be used simultaneously. Please use only one of them.";
9696
public static NO_DEVICES_EMULATORS_FOUND_FOR_OPTIONS = "Unable to find device or emulator for specified options.";
97+
public static UNSUPPORTED_DEVICE_OS_FOR_DEBUGGING = "Unsupported device OS for debugging";
9798
}
9899

99100
export const enum NativePlatformStatus {

lib/services/debug-service.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { platform } from "os";
22
import { EventEmitter } from "events";
3-
import { CONNECTION_ERROR_EVENT_NAME } from "../constants";
3+
import { CONNECTION_ERROR_EVENT_NAME, DebugCommandErrors } from "../constants";
44
import { CONNECTED_STATUS } from "../common/constants";
55

66
export class DebugService extends EventEmitter implements IDebugService {
@@ -73,7 +73,7 @@ export class DebugService extends EventEmitter implements IDebugService {
7373
} else if (this.$mobileHelper.isAndroidPlatform(platform)) {
7474
this._platformDebugServices[device.deviceInfo.identifier] = this.$injector.resolve("androidDebugService", { device });
7575
} else {
76-
this.$errors.failWithoutHelp(`Could not get debug service for platform: ${platform}`);
76+
this.$errors.failWithoutHelp(DebugCommandErrors.UNSUPPORTED_DEVICE_OS_FOR_DEBUGGING);
7777
}
7878

7979
this.attachConnectionErrorHandlers(this._platformDebugServices[device.deviceInfo.identifier]);

lib/services/livesync/livesync-service.ts

+30-19
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi
155155

156156
try {
157157
await deviceAppData.device.applicationManager.stopApplication(applicationId, projectData.projectName);
158+
// Now that we've stopped the application we know it isn't started, so set debugOptions.start to false
159+
// so that it doesn't default to true in attachDebugger method
160+
debugOptions = debugOptions || {};
161+
debugOptions.start = false;
158162
} catch (err) {
159163
this.$logger.trace("Could not stop application during debug livesync. Will try to restart app instead.", err);
160164
if ((err.message || err) === "Could not find developer disk image") {
@@ -176,8 +180,15 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi
176180

177181
public async attachDebugger(settings: IAttachDebuggerOptions): Promise<void> {
178182
// Default values
179-
settings.debugOptions.chrome = settings.debugOptions.chrome === undefined ? true : settings.debugOptions.chrome;
180-
settings.debugOptions.start = settings.debugOptions.start === undefined ? true : settings.debugOptions.start;
183+
if (settings.debugOptions) {
184+
settings.debugOptions.chrome = settings.debugOptions.chrome === undefined ? true : settings.debugOptions.chrome;
185+
settings.debugOptions.start = settings.debugOptions.start === undefined ? true : settings.debugOptions.start;
186+
} else {
187+
settings.debugOptions = {
188+
chrome: true,
189+
start: true
190+
};
191+
}
181192

182193
const projectData = this.$projectDataService.getProjectData(settings.projectDir);
183194
let debugData = this.$debugDataService.createDebugData(projectData, { device: settings.deviceIdentifier });
@@ -225,25 +236,25 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi
225236
this.$errors.failWithoutHelp(`Couldn't enable debugging for ${deviceOption.deviceIdentifier}`);
226237
}
227238

228-
currentDeviceDescriptor.debugggingEnabled = true;
229-
const currentDeviceInstance = this.$devicesService.getDeviceByIdentifier(deviceOption.deviceIdentifier);
230-
const attachDebuggerOptions: IAttachDebuggerOptions = {
231-
deviceIdentifier: deviceOption.deviceIdentifier,
232-
isEmulator: currentDeviceInstance.isEmulator,
233-
outputPath: currentDeviceDescriptor.outputPath,
234-
platform: currentDeviceInstance.deviceInfo.platform,
235-
projectDir: debuggingAdditionalOptions.projectDir,
236-
debugOptions: deviceOption.debugOptions
237-
};
239+
currentDeviceDescriptor.debugggingEnabled = true;
240+
const currentDeviceInstance = this.$devicesService.getDeviceByIdentifier(deviceOption.deviceIdentifier);
241+
const attachDebuggerOptions: IAttachDebuggerOptions = {
242+
deviceIdentifier: deviceOption.deviceIdentifier,
243+
isEmulator: currentDeviceInstance.isEmulator,
244+
outputPath: currentDeviceDescriptor.outputPath,
245+
platform: currentDeviceInstance.deviceInfo.platform,
246+
projectDir: debuggingAdditionalOptions.projectDir,
247+
debugOptions: deviceOption.debugOptions
248+
};
238249

239-
try {
240-
await this.attachDebugger(attachDebuggerOptions);
241-
} catch (err) {
242-
this.$logger.trace("Couldn't attach debugger, will modify options and try again.", err);
243-
attachDebuggerOptions.debugOptions.start = false;
244-
await this.attachDebugger(attachDebuggerOptions);
245-
}
250+
try {
251+
await this.attachDebugger(attachDebuggerOptions);
252+
} catch (err) {
253+
this.$logger.trace("Couldn't attach debugger, will modify options and try again.", err);
254+
attachDebuggerOptions.debugOptions.start = false;
255+
await this.attachDebugger(attachDebuggerOptions);
246256
}
257+
}
247258

248259
private async enableDebuggingCore(deviceOption: IEnableDebuggingDeviceOptions, debuggingAdditionalOptions: IDebuggingAdditionalOptions): Promise<void> {
249260
const liveSyncProcessInfo: ILiveSyncProcessInfo = this.liveSyncProcessesInfo[debuggingAdditionalOptions.projectDir];

test/services/android-debug-service.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,10 @@ class AndroidDebugServiceInheritor extends AndroidDebugService {
99
constructor(protected $devicesService: Mobile.IDevicesService,
1010
$errors: IErrors,
1111
$logger: ILogger,
12-
$config: IConfiguration,
1312
$androidDeviceDiscovery: Mobile.IDeviceDiscovery,
1413
$androidProcessService: Mobile.IAndroidProcessService,
1514
$net: INet) {
16-
super($devicesService, $errors, $logger, $config, $androidDeviceDiscovery, $androidProcessService, $net);
15+
super(<any>{}, $devicesService, $errors, $logger, $androidDeviceDiscovery, $androidProcessService, $net);
1716
}
1817

1918
public getChromeDebugUrl(debugOptions: IDebugOptions, port: number): string {
@@ -26,7 +25,6 @@ const createTestInjector = (): IInjector => {
2625
testInjector.register("devicesService", {});
2726
testInjector.register("errors", stubs.ErrorsStub);
2827
testInjector.register("logger", stubs.LoggerStub);
29-
testInjector.register("config", {});
3028
testInjector.register("androidDeviceDiscovery", {});
3129
testInjector.register("androidProcessService", {});
3230
testInjector.register("net", {});

test/services/debug-service.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as stubs from "../stubs";
44
import { assert } from "chai";
55
import { EventEmitter } from "events";
66
import * as constants from "../../lib/common/constants";
7-
import { CONNECTION_ERROR_EVENT_NAME } from "../../lib/constants";
7+
import { CONNECTION_ERROR_EVENT_NAME, DebugCommandErrors } from "../../lib/constants";
88

99
const fakeChromeDebugUrl = "fakeChromeDebugUrl";
1010
class PlatformDebugService extends EventEmitter /* implements IPlatformDebugService */ {
@@ -141,7 +141,7 @@ describe("debugService", () => {
141141
const testData = getDefaultTestData();
142142
testData.deviceInformation.deviceInfo.platform = "WP8";
143143

144-
await assertIsRejected(testData, "Unsupported device OS:");
144+
await assertIsRejected(testData, DebugCommandErrors.UNSUPPORTED_DEVICE_OS_FOR_DEBUGGING);
145145
});
146146

147147
it("when trying to debug on iOS Simulator on macOS, debug-brk is passed, but pathToAppPackage is not", async () => {

test/services/ios-debug-service.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,15 @@ class IOSDebugServiceInheritor extends IOSDebugService {
1010
$platformService: IPlatformService,
1111
$iOSEmulatorServices: Mobile.IEmulatorPlatformServices,
1212
$childProcess: IChildProcess,
13+
$hostInfo: IHostInfo,
1314
$logger: ILogger,
1415
$errors: IErrors,
1516
$npmInstallationManager: INpmInstallationManager,
1617
$iOSNotification: IiOSNotification,
1718
$iOSSocketRequestExecutor: IiOSSocketRequestExecutor,
1819
$processService: IProcessService,
1920
$socketProxyFactory: ISocketProxyFactory) {
20-
super($devicesService, $platformService, $iOSEmulatorServices, $childProcess, $logger, $errors,
21+
super(<any>{}, $devicesService, $platformService, $iOSEmulatorServices, $childProcess, $hostInfo, $logger, $errors,
2122
$npmInstallationManager, $iOSNotification, $iOSSocketRequestExecutor, $processService, $socketProxyFactory);
2223
}
2324

@@ -35,6 +36,7 @@ const createTestInjector = (): IInjector => {
3536

3637
testInjector.register("errors", stubs.ErrorsStub);
3738
testInjector.register("logger", stubs.LoggerStub);
39+
testInjector.register("hostInfo", {});
3840
testInjector.register("npmInstallationManager", {});
3941
testInjector.register("iOSNotification", {});
4042
testInjector.register("iOSSocketRequestExecutor", {});

0 commit comments

Comments
 (0)