Skip to content

Commit 1cd6874

Browse files
TsvetanMilanovTsvetanMilanov
committed
Fix PR comments
1 parent 6ca7280 commit 1cd6874

File tree

10 files changed

+69
-75
lines changed

10 files changed

+69
-75
lines changed

‎lib/commands/debug.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export abstract class DebugPlatformCommand implements ICommand {
1515
protected $projectData: IProjectData,
1616
protected $options: IOptions,
1717
protected $platformsData: IPlatformsData) {
18-
this.$projectData.initializeProjectData(this.$options.path);
18+
this.$projectData.initializeProjectData();
1919
}
2020

2121
public async execute(args: string[]): Promise<void> {

‎lib/common

‎lib/device-sockets/ios/socket-proxy-factory.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { EventEmitter } from "events";
22
import { CONNECTION_ERROR_EVENT_NAME } from "../../constants";
33
import { PacketStream } from "./packet-stream";
4-
import { getAvailablePort } from "../../common/helpers";
54
import * as net from "net";
65
import * as ws from "ws";
76
import temp = require("temp");
@@ -10,7 +9,8 @@ export class SocketProxyFactory extends EventEmitter implements ISocketProxyFact
109
constructor(private $logger: ILogger,
1110
private $errors: IErrors,
1211
private $config: IConfiguration,
13-
private $options: IOptions) {
12+
private $options: IOptions,
13+
private $net: INet) {
1414
super();
1515
}
1616

@@ -70,7 +70,7 @@ export class SocketProxyFactory extends EventEmitter implements ISocketProxyFact
7070

7171
public async createWebSocketProxy(factory: () => Promise<net.Socket>): Promise<ws.Server> {
7272
// NOTE: We will try to provide command line options to select ports, at least on the localhost.
73-
const localPort = await getAvailablePort(8080);
73+
const localPort = await this.$net.getAvailablePortInRange(8080);
7474

7575
this.$logger.info("\nSetting up debugger proxy...\nPress Ctrl + C to terminate, or disconnect.\n");
7676

‎lib/services/android-debug-service.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { sleep, getAvailablePort } from "../common/helpers";
1+
import { sleep } from "../common/helpers";
22
import { ChildProcess } from "child_process";
33
import { DebugServiceBase } from "./debug-service-base";
44

@@ -23,7 +23,8 @@ class AndroidDebugService extends DebugServiceBase implements IPlatformDebugServ
2323
private $logger: ILogger,
2424
private $config: IConfiguration,
2525
private $androidDeviceDiscovery: Mobile.IDeviceDiscovery,
26-
private $androidProcessService: Mobile.IAndroidProcessService) {
26+
private $androidProcessService: Mobile.IAndroidProcessService,
27+
private $net: INet) {
2728
super();
2829
}
2930

@@ -69,7 +70,7 @@ class AndroidDebugService extends DebugServiceBase implements IPlatformDebugServ
6970
if (match) {
7071
port = parseInt(match[1]);
7172
} else {
72-
port = await getAvailablePort(40000);
73+
port = await this.$net.getAvailablePortInRange(40000);
7374

7475
await this.unixSocketForward(port, `${unixSocketName}`);
7576
}
@@ -127,7 +128,7 @@ class AndroidDebugService extends DebugServiceBase implements IPlatformDebugServ
127128
let startDebuggerCommand = ["am", "broadcast", "-a", `\"${packageName}-debug\"`, "--ez", "enable", "true"];
128129
await this.device.adb.executeShellCommand(startDebuggerCommand);
129130

130-
if (debugOptions.client) {
131+
if (debugOptions.chrome) {
131132
let port = await this.getForwardedLocalDebugPortForPackageName(deviceId, packageName);
132133
return `chrome-devtools://devtools/bundled/inspector.html?experiments=true&ws=localhost:${port}`;
133134
}

‎lib/services/debug-data-service.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
export class DebugDataService implements IDebugDataService {
22
constructor(private $projectData: IProjectData,
3-
private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
43
private $platformService: IPlatformService,
5-
private $platformsData: IPlatformsData) { }
4+
private $platformsData: IPlatformsData,
5+
private $mobileHelper: Mobile.IMobileHelper) { }
66

77
public createDebugData(debugService: IPlatformDebugService, options: IOptions, buildConfig: IBuildConfig): IDebugData {
88
this.$projectData.initializeProjectData(options.path);
@@ -16,26 +16,24 @@ export class DebugDataService implements IDebugDataService {
1616
}
1717

1818
private getPathToAppPackage(debugService: IPlatformDebugService, options: IOptions, buildConfig: IBuildConfig): string {
19-
if (debugService.platform === this.$devicePlatformsConstants.Android) {
19+
if (this.$mobileHelper.isAndroidPlatform(debugService.platform)) {
2020
if (!options.start && !options.emulator) {
2121
const platformData = this.getPlatformData(debugService);
2222

2323
return this.$platformService.getLatestApplicationPackageForDevice(platformData, buildConfig).packageName;
24-
} else {
25-
return null;
2624
}
27-
} else if (debugService.platform === this.$devicePlatformsConstants.iOS) {
25+
} else if (this.$mobileHelper.isiOSPlatform(debugService.platform)) {
2826
if (options.emulator) {
2927
const platformData = this.getPlatformData(debugService);
3028

3129
return this.$platformService.getLatestApplicationPackageForEmulator(platformData, buildConfig).packageName;
32-
} else {
33-
return null;
3430
}
3531
}
32+
33+
return null;
3634
}
3735

38-
protected getPlatformData(debugService: IPlatformDebugService): IPlatformData {
36+
private getPlatformData(debugService: IPlatformDebugService): IPlatformData {
3937
return this.$platformsData.getPlatformData(debugService.platform, this.$projectData);
4038
}
4139
}

‎lib/services/debug-service-base.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import { EventEmitter } from "events";
22

33
export abstract class DebugServiceBase extends EventEmitter implements IPlatformDebugService {
4-
constructor() {
5-
super();
6-
}
7-
84
public abstract get platform(): string;
95

106
public abstract async debug(debugData: IDebugData, debugOptions: IDebugOptions): Promise<string[]>;

‎lib/services/debug-service.ts

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,34 +2,41 @@ import { platform } from "os";
22
import { EventEmitter } from "events";
33
import { CONNECTION_ERROR_EVENT_NAME } from "../constants";
44

5+
// This service can't implement IDebugService because
6+
// the debug method returns only one result.
57
class DebugService extends EventEmitter {
68
constructor(private $devicesService: Mobile.IDevicesService,
79
private $androidDebugService: IPlatformDebugService,
810
private $iOSDebugService: IPlatformDebugService,
911
private $errors: IErrors,
1012
private $hostInfo: IHostInfo,
11-
private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants) {
13+
private $mobileHelper: Mobile.IMobileHelper) {
1214
super();
15+
this.attachConnectionErrorHandlers();
1316
}
1417

1518
public async debug(debugData: IDebugData, options: IDebugOptions): Promise<string> {
1619
const device = this.$devicesService.getDeviceByIdentifier(debugData.deviceIdentifier);
1720
const debugService = this.getDebugService(device);
1821

19-
debugService.on(CONNECTION_ERROR_EVENT_NAME, (e: Error) => this.emit(CONNECTION_ERROR_EVENT_NAME, e));
20-
2122
if (!device) {
2223
this.$errors.failWithoutHelp(`Can't find device with identifier ${debugData.deviceIdentifier}`);
2324
}
2425

25-
const debugOptions: IDebugOptions = _.merge({}, options || {});
26+
if (!(await device.applicationManager.isApplicationInstalled(debugData.applicationIdentifier))) {
27+
this.$errors.failWithoutHelp(`The application ${debugData.applicationIdentifier} is not installed on device with identifier ${debugData.deviceIdentifier}.`);
28+
}
29+
30+
const debugOptions: IDebugOptions = _.merge({}, options);
2631
debugOptions.start = true;
2732

2833
// TODO: Check if app is running.
34+
// For now we can only check if app is running on Android.
35+
// After we find a way to check on iOS we should use it here.
2936
const isAppRunning = true;
3037
let result: string[];
31-
if (device.deviceInfo.platform === this.$devicePlatformsConstants.iOS) {
32-
debugOptions.chrome = true;
38+
debugOptions.chrome = !debugOptions.client;
39+
if (this.$mobileHelper.isiOSPlatform(device.deviceInfo.platform)) {
3340
if (device.isEmulator && !debugData.pathToAppPackage) {
3441
this.$errors.failWithoutHelp("To debug on iOS simulator you need to provide path to the app package.");
3542
}
@@ -45,21 +52,27 @@ class DebugService extends EventEmitter {
4552
}
4653

4754
result = await debugService.debug(debugData, debugOptions);
48-
} else if (device.deviceInfo.platform === this.$devicePlatformsConstants.Android) {
49-
debugOptions.client = true;
55+
} else if (this.$mobileHelper.isAndroidPlatform(device.deviceInfo.platform)) {
5056
result = await debugService.debug(debugData, debugOptions);
5157
}
5258

5359
return _.first(result);
5460
}
5561

5662
private getDebugService(device: Mobile.IDevice): IPlatformDebugService {
57-
if (device.deviceInfo.platform === this.$devicePlatformsConstants.iOS) {
63+
if (this.$mobileHelper.isiOSPlatform(device.deviceInfo.platform)) {
5864
return this.$iOSDebugService;
59-
} else if (device.deviceInfo.platform === this.$devicePlatformsConstants.Android) {
65+
} else if (this.$mobileHelper.isAndroidPlatform(device.deviceInfo.platform)) {
6066
return this.$androidDebugService;
6167
}
6268
}
69+
70+
private attachConnectionErrorHandlers() {
71+
let connectionErrorHandler = (e: Error) => this.emit(CONNECTION_ERROR_EVENT_NAME, e);
72+
connectionErrorHandler = connectionErrorHandler.bind(this);
73+
this.$androidDebugService.on(CONNECTION_ERROR_EVENT_NAME, connectionErrorHandler);
74+
this.$iOSDebugService.on(CONNECTION_ERROR_EVENT_NAME, connectionErrorHandler);
75+
}
6376
}
6477

6578
$injector.register("debugService", DebugService);

‎lib/services/ios-debug-service.ts

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -50,27 +50,23 @@ class IOSDebugService extends DebugServiceBase implements IPlatformDebugService
5050
}
5151

5252
if (debugOptions.emulator) {
53-
if (debugOptions.debugBrk) {
54-
return [await this.emulatorDebugBrk(debugData, true, debugOptions)];
55-
} else if (debugOptions.start) {
53+
if (debugOptions.start) {
5654
return [await this.emulatorStart(debugData, debugOptions)];
5755
} else {
58-
return [await this.emulatorDebugBrk(debugData, false, debugOptions)];
56+
return [await this.emulatorDebugBrk(debugData, debugOptions)];
5957
}
6058
} else {
61-
if (debugOptions.debugBrk) {
62-
return this.deviceDebugBrk(debugData, true, debugOptions);
63-
} else if (debugOptions.start) {
59+
if (debugOptions.start) {
6460
return this.deviceStart(debugData, debugOptions);
6561
} else {
66-
return this.deviceDebugBrk(debugData, false, debugOptions);
62+
return this.deviceDebugBrk(debugData, debugOptions);
6763
}
6864
}
6965
}
7066

7167
public async debugStart(debugData: IDebugData, debugOptions: IDebugOptions): Promise<void> {
7268
await this.$devicesService.initialize({ platform: this.platform, deviceId: debugData.deviceIdentifier });
73-
const action = async (device: Mobile.IiOSDevice) => await device.isEmulator ? this.emulatorDebugBrk(debugData, false, debugOptions) : this.debugBrkCore(device, debugData, debugOptions);
69+
const action = async (device: Mobile.IiOSDevice) => device.isEmulator ? await this.emulatorDebugBrk(debugData, debugOptions) : await this.debugBrkCore(device, debugData, debugOptions);
7470
await this.$devicesService.execute(action, this.getCanExecuteAction(debugData.deviceIdentifier));
7571
}
7672

@@ -95,8 +91,8 @@ class IOSDebugService extends DebugServiceBase implements IPlatformDebugService
9591
}
9692
}
9793

98-
private async emulatorDebugBrk(debugData: IDebugData, shouldBreak: boolean, debugOptions: IDebugOptions): Promise<string> {
99-
let args = shouldBreak ? "--nativescript-debug-brk" : "--nativescript-debug-start";
94+
private async emulatorDebugBrk(debugData: IDebugData, debugOptions: IDebugOptions): Promise<string> {
95+
let args = debugOptions.debugBrk ? "--nativescript-debug-brk" : "--nativescript-debug-start";
10096
let child_process = await this.$iOSEmulatorServices.runApplicationOnEmulator(debugData.pathToAppPackage, {
10197
waitForDebugger: true,
10298
captureStdin: true,
@@ -136,11 +132,11 @@ class IOSDebugService extends DebugServiceBase implements IPlatformDebugService
136132
return result;
137133
}
138134

139-
private async deviceDebugBrk(debugData: IDebugData, shouldBreak: boolean, debugOptions: IDebugOptions): Promise<string[]> {
135+
private async deviceDebugBrk(debugData: IDebugData, debugOptions: IDebugOptions): Promise<string[]> {
140136
await this.$devicesService.initialize({ platform: this.platform, deviceId: debugData.deviceIdentifier });
141137
const action = async (device: iOSDevice.IOSDevice) => {
142138
if (device.isEmulator) {
143-
return await this.emulatorDebugBrk(debugData, shouldBreak, debugOptions);
139+
return await this.emulatorDebugBrk(debugData, debugOptions);
144140
}
145141

146142
const runOptions: IRunPlatformOptions = {
@@ -151,27 +147,27 @@ class IOSDebugService extends DebugServiceBase implements IPlatformDebugService
151147
// we intentionally do not wait on this here, because if we did, we'd miss the AppLaunching notification
152148
let startApplicationAction = this.$platformService.startApplication(this.platform, runOptions, debugData.applicationIdentifier);
153149

154-
const result = await this.debugBrkCore(device, debugData, debugOptions, shouldBreak);
150+
const result = await this.debugBrkCore(device, debugData, debugOptions);
155151

156152
await startApplicationAction;
157153

158154
return result;
159155
};
160156

161-
const result = await this.$devicesService.execute(action, this.getCanExecuteAction(debugData.deviceIdentifier));
162-
return _.map(result, r => r.result);
157+
const results = await this.$devicesService.execute(action, this.getCanExecuteAction(debugData.deviceIdentifier));
158+
return _.map(results, r => r.result);
163159
}
164160

165-
private async debugBrkCore(device: Mobile.IiOSDevice, debugData: IDebugData, debugOptions: IDebugOptions, shouldBreak?: boolean): Promise<string> {
166-
await this.$iOSSocketRequestExecutor.executeLaunchRequest(device.deviceInfo.identifier, TIMEOUT_SECONDS, TIMEOUT_SECONDS, debugData.applicationIdentifier, shouldBreak);
161+
private async debugBrkCore(device: Mobile.IiOSDevice, debugData: IDebugData, debugOptions: IDebugOptions): Promise<string> {
162+
await this.$iOSSocketRequestExecutor.executeLaunchRequest(device.deviceInfo.identifier, TIMEOUT_SECONDS, TIMEOUT_SECONDS, debugData.applicationIdentifier, debugOptions.debugBrk);
167163
return this.wireDebuggerClient(debugData, debugOptions, device);
168164
}
169165

170166
private async deviceStart(debugData: IDebugData, debugOptions: IDebugOptions): Promise<string[]> {
171167
await this.$devicesService.initialize({ platform: this.platform, deviceId: debugData.deviceIdentifier });
172168
const action = async (device: Mobile.IiOSDevice) => device.isEmulator ? await this.emulatorStart(debugData, debugOptions) : await this.deviceStartCore(device, debugData, debugOptions);
173-
const result = await this.$devicesService.execute(action, this.getCanExecuteAction(debugData.deviceIdentifier));
174-
return _.map(result, r => r.result);
169+
const results = await this.$devicesService.execute(action, this.getCanExecuteAction(debugData.deviceIdentifier));
170+
return _.map(results, r => r.result);
175171
}
176172

177173
private async deviceStartCore(device: Mobile.IiOSDevice, debugData: IDebugData, debugOptions: IDebugOptions): Promise<string> {

‎lib/services/test-execution-service.ts

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -130,33 +130,23 @@ class TestExecutionService implements ITestExecutionService {
130130
this.$errors.failWithoutHelp("Verify that listed files are well-formed and try again the operation.");
131131
}
132132

133-
if (this.$options.debugBrk) {
134-
const debugService = this.getDebugService(platform); const deployOptions: IDeployPlatformOptions = {
135-
clean: this.$options.clean,
136-
device: this.$options.device,
137-
emulator: this.$options.emulator,
138-
projectDir: this.$options.path,
139-
platformTemplate: this.$options.platformTemplate,
140-
release: this.$options.release,
141-
provision: this.$options.provision,
142-
teamId: this.$options.teamId
143-
};
133+
const deployOptions: IDeployPlatformOptions = {
134+
clean: this.$options.clean,
135+
device: this.$options.device,
136+
emulator: this.$options.emulator,
137+
projectDir: this.$options.path,
138+
platformTemplate: this.$options.platformTemplate,
139+
release: this.$options.release,
140+
provision: this.$options.provision,
141+
teamId: this.$options.teamId
142+
};
144143

144+
if (this.$options.debugBrk) {
145+
const debugService = this.getDebugService(platform);
145146
const buildConfig: IBuildConfig = _.merge({ buildForDevice: this.$options.forDevice }, deployOptions);
146147
const debugData = this.$debugDataService.createDebugData(debugService, this.$options, buildConfig);
147148
await debugService.debug(debugData, this.$options);
148149
} else {
149-
const deployOptions: IDeployPlatformOptions = {
150-
clean: this.$options.clean,
151-
device: this.$options.device,
152-
emulator: this.$options.emulator,
153-
projectDir: this.$options.path,
154-
platformTemplate: this.$options.platformTemplate,
155-
release: this.$options.release,
156-
provision: this.$options.provision,
157-
teamId: this.$options.teamId
158-
};
159-
160150
await this.$platformService.deployPlatform(platform, appFilesUpdaterOptions, deployOptions, projectData, { provision: this.$options.provision, sdk: this.$options.sdk });
161151
await this.$usbLiveSyncService.liveSync(platform, projectData);
162152
}

‎test/debug.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ function createTestInjector(): IInjector {
4444
testInjector.register("xmlValidator", {});
4545
testInjector.register("npm", {});
4646
testInjector.register("debugDataService", {
47-
createDebugData: () => { return {}; }
47+
createDebugData: () => ({})
4848
});
4949
testInjector.register("androidEmulatorServices", {});
5050
testInjector.register("adb", AndroidDebugBridge);

0 commit comments

Comments
 (0)