Skip to content

Commit 873d82b

Browse files
Merge pull request #4573 from NativeScript/vladimirov/fix-android-logs-2
fix: logcat process is not restarted in some cases
2 parents a28b41b + 0468853 commit 873d82b

File tree

2 files changed

+57
-27
lines changed

2 files changed

+57
-27
lines changed

lib/common/mobile/android/logcat-helper.ts

+10-5
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ export class LogcatHelper implements Mobile.ILogcatHelper {
3939

4040
logcatStream.on("close", (code: number) => {
4141
try {
42-
this.stop(deviceIdentifier);
42+
this.forceStop(deviceIdentifier);
43+
4344
if (code !== 0) {
4445
this.$logger.trace("ADB process exited with code " + code.toString());
4546
}
@@ -76,13 +77,17 @@ export class LogcatHelper implements Mobile.ILogcatHelper {
7677
*/
7778
public stop(deviceIdentifier: string): void {
7879
if (this.mapDevicesLoggingData[deviceIdentifier] && !this.mapDevicesLoggingData[deviceIdentifier].keepSingleProcess) {
79-
this.mapDevicesLoggingData[deviceIdentifier].loggingProcess.removeAllListeners();
80-
this.mapDevicesLoggingData[deviceIdentifier].loggingProcess.kill("SIGINT");
81-
this.mapDevicesLoggingData[deviceIdentifier].lineStream.removeAllListeners();
82-
delete this.mapDevicesLoggingData[deviceIdentifier];
80+
this.forceStop(deviceIdentifier);
8381
}
8482
}
8583

84+
private forceStop(deviceIdentifier: string): void {
85+
this.mapDevicesLoggingData[deviceIdentifier].loggingProcess.removeAllListeners();
86+
this.mapDevicesLoggingData[deviceIdentifier].loggingProcess.kill("SIGINT");
87+
this.mapDevicesLoggingData[deviceIdentifier].lineStream.removeAllListeners();
88+
delete this.mapDevicesLoggingData[deviceIdentifier];
89+
}
90+
8691
private async getLogcatStream(deviceIdentifier: string, pid?: string) {
8792
const device = await this.$devicesService.getDevice(deviceIdentifier);
8893
const minAndroidWithLogcatPidSupport = "7.0.0";

lib/common/test/unit-tests/mobile/android/logcat-helper.ts

+47-22
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,39 @@ import { Yok } from "../../../../yok";
33
import { assert } from "chai";
44
import * as path from "path";
55
import * as childProcess from "child_process";
6+
import * as fileSystem from "fs";
7+
import { EventEmitter } from "events";
8+
import stream = require("stream");
9+
10+
class ChildProcessMockInstance extends EventEmitter {
11+
public stdout: stream.Readable;
12+
public stderr: any;
13+
public processKillCallCount = 0;
14+
15+
constructor(pathToSample: string) {
16+
super();
17+
this.stdout = fileSystem.createReadStream(pathToSample);
18+
this.stderr = new EventEmitter();
19+
}
20+
21+
public kill(): void {
22+
this.processKillCallCount++;
23+
}
24+
}
625

726
class ChildProcessStub {
827
public processSpawnCallCount = 0;
9-
public processKillCallCount = 0;
1028
public adbProcessArgs: string[] = [];
11-
private isWin = /^win/.test(process.platform);
29+
public lastSpawnedProcess: ChildProcessMockInstance = null;
1230

1331
public spawn(command: string, args?: string[], options?: any): childProcess.ChildProcess {
1432
this.adbProcessArgs = args;
1533
this.processSpawnCallCount++;
16-
let pathToExecutable = "";
17-
let shell = "";
18-
if (this.isWin) {
19-
pathToExecutable = "type";
20-
shell = "cmd";
21-
} else {
22-
pathToExecutable = "cat";
23-
}
24-
pathToExecutable = path.join(pathToExecutable);
2534
const pathToSample = path.join(__dirname, "valid-sample.txt");
26-
const spawnedProcess = childProcess.spawn(pathToExecutable, [pathToSample], { shell });
27-
const spawnedProcessKill = spawnedProcess.kill;
28-
spawnedProcess.kill = (signal: string) => {
29-
this.processKillCallCount++;
30-
spawnedProcessKill.call(spawnedProcessKill, signal);
31-
};
32-
33-
return spawnedProcess;
35+
36+
this.lastSpawnedProcess = new ChildProcessMockInstance(pathToSample);
37+
38+
return <any>this.lastSpawnedProcess;
3439
}
3540
}
3641

@@ -207,7 +212,7 @@ describe("logcat-helper", () => {
207212
await logcatHelper.stop(validIdentifier);
208213
await logcatHelper.stop(validIdentifier);
209214

210-
assert.equal(childProcessStub.processKillCallCount, 1);
215+
assert.equal(childProcessStub.lastSpawnedProcess.processKillCallCount, 1);
211216
});
212217

213218
it("should not kill the process if started with keepSingleProcess", async () => {
@@ -220,7 +225,7 @@ describe("logcat-helper", () => {
220225

221226
await logcatHelper.stop(validIdentifier);
222227
await logcatHelper.stop(validIdentifier);
223-
assert.equal(childProcessStub.processKillCallCount, 0);
228+
assert.equal(childProcessStub.lastSpawnedProcess.processKillCallCount, 0);
224229
});
225230

226231
it("should do nothing if called without start", async () => {
@@ -229,7 +234,27 @@ describe("logcat-helper", () => {
229234
await logcatHelper.stop(validIdentifier);
230235

231236
assert.equal(childProcessStub.processSpawnCallCount, 0);
232-
assert.equal(childProcessStub.processKillCallCount, 0);
233237
});
238+
239+
for (const keepSingleProcess of [false, true]) {
240+
it(`should clear the device identifier cache when the logcat process is killed manually and keepSingleProcess is ${keepSingleProcess}`, async () => {
241+
const logcatHelper = injector.resolve<LogcatHelper>("logcatHelper");
242+
243+
await logcatHelper.start({
244+
deviceIdentifier: validIdentifier,
245+
keepSingleProcess
246+
});
247+
248+
assert.equal(childProcessStub.processSpawnCallCount, 1);
249+
250+
childProcessStub.lastSpawnedProcess.emit("close");
251+
252+
await logcatHelper.start({
253+
deviceIdentifier: validIdentifier
254+
});
255+
256+
assert.equal(childProcessStub.processSpawnCallCount, 2);
257+
});
258+
}
234259
});
235260
});

0 commit comments

Comments
 (0)