Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: logcat process is not restarted in some cases #4573

Merged
merged 1 commit into from
May 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 10 additions & 5 deletions lib/common/mobile/android/logcat-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ export class LogcatHelper implements Mobile.ILogcatHelper {

logcatStream.on("close", (code: number) => {
try {
this.stop(deviceIdentifier);
this.forceStop(deviceIdentifier);

if (code !== 0) {
this.$logger.trace("ADB process exited with code " + code.toString());
}
Expand Down Expand Up @@ -76,13 +77,17 @@ export class LogcatHelper implements Mobile.ILogcatHelper {
*/
public stop(deviceIdentifier: string): void {
if (this.mapDevicesLoggingData[deviceIdentifier] && !this.mapDevicesLoggingData[deviceIdentifier].keepSingleProcess) {
this.mapDevicesLoggingData[deviceIdentifier].loggingProcess.removeAllListeners();
this.mapDevicesLoggingData[deviceIdentifier].loggingProcess.kill("SIGINT");
this.mapDevicesLoggingData[deviceIdentifier].lineStream.removeAllListeners();
delete this.mapDevicesLoggingData[deviceIdentifier];
this.forceStop(deviceIdentifier);
}
}

private forceStop(deviceIdentifier: string): void {
this.mapDevicesLoggingData[deviceIdentifier].loggingProcess.removeAllListeners();
this.mapDevicesLoggingData[deviceIdentifier].loggingProcess.kill("SIGINT");
this.mapDevicesLoggingData[deviceIdentifier].lineStream.removeAllListeners();
delete this.mapDevicesLoggingData[deviceIdentifier];
}

private async getLogcatStream(deviceIdentifier: string, pid?: string) {
const device = await this.$devicesService.getDevice(deviceIdentifier);
const minAndroidWithLogcatPidSupport = "7.0.0";
Expand Down
69 changes: 47 additions & 22 deletions lib/common/test/unit-tests/mobile/android/logcat-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,39 @@ import { Yok } from "../../../../yok";
import { assert } from "chai";
import * as path from "path";
import * as childProcess from "child_process";
import * as fileSystem from "fs";
import { EventEmitter } from "events";
import stream = require("stream");

class ChildProcessMockInstance extends EventEmitter {
public stdout: stream.Readable;
public stderr: any;
public processKillCallCount = 0;

constructor(pathToSample: string) {
super();
this.stdout = fileSystem.createReadStream(pathToSample);
this.stderr = new EventEmitter();
}

public kill(): void {
this.processKillCallCount++;
}
}

class ChildProcessStub {
public processSpawnCallCount = 0;
public processKillCallCount = 0;
public adbProcessArgs: string[] = [];
private isWin = /^win/.test(process.platform);
public lastSpawnedProcess: ChildProcessMockInstance = null;

public spawn(command: string, args?: string[], options?: any): childProcess.ChildProcess {
this.adbProcessArgs = args;
this.processSpawnCallCount++;
let pathToExecutable = "";
let shell = "";
if (this.isWin) {
pathToExecutable = "type";
shell = "cmd";
} else {
pathToExecutable = "cat";
}
pathToExecutable = path.join(pathToExecutable);
const pathToSample = path.join(__dirname, "valid-sample.txt");
const spawnedProcess = childProcess.spawn(pathToExecutable, [pathToSample], { shell });
const spawnedProcessKill = spawnedProcess.kill;
spawnedProcess.kill = (signal: string) => {
this.processKillCallCount++;
spawnedProcessKill.call(spawnedProcessKill, signal);
};

return spawnedProcess;

this.lastSpawnedProcess = new ChildProcessMockInstance(pathToSample);

return <any>this.lastSpawnedProcess;
}
}

Expand Down Expand Up @@ -207,7 +212,7 @@ describe("logcat-helper", () => {
await logcatHelper.stop(validIdentifier);
await logcatHelper.stop(validIdentifier);

assert.equal(childProcessStub.processKillCallCount, 1);
assert.equal(childProcessStub.lastSpawnedProcess.processKillCallCount, 1);
});

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

await logcatHelper.stop(validIdentifier);
await logcatHelper.stop(validIdentifier);
assert.equal(childProcessStub.processKillCallCount, 0);
assert.equal(childProcessStub.lastSpawnedProcess.processKillCallCount, 0);
});

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

assert.equal(childProcessStub.processSpawnCallCount, 0);
assert.equal(childProcessStub.processKillCallCount, 0);
});

for (const keepSingleProcess of [false, true]) {
it(`should clear the device identifier cache when the logcat process is killed manually and keepSingleProcess is ${keepSingleProcess}`, async () => {
const logcatHelper = injector.resolve<LogcatHelper>("logcatHelper");

await logcatHelper.start({
deviceIdentifier: validIdentifier,
keepSingleProcess
});

assert.equal(childProcessStub.processSpawnCallCount, 1);

childProcessStub.lastSpawnedProcess.emit("close");

await logcatHelper.start({
deviceIdentifier: validIdentifier
});

assert.equal(childProcessStub.processSpawnCallCount, 2);
});
}
});
});