Skip to content

Commit

Permalink
fix: ADB.pidof integration issue with grep and CRLF edge case (#927)
Browse files Browse the repository at this point in the history 
* fix: using adb logcat options compatible with API level 19
* fix: added basic error handling for ADB.install
* using a dedicated module for sanitizing filenames
* fix: a stricter way to check if uninstall is required
  • Loading branch information
@noomorph
noomorph authored Sep 26, 2018
1 parent 6cdbca2 commit 3e11f66
Show file tree
Hide file tree
Showing 14 changed files with 333 additions and 316 deletions.
1 change: 1 addition & 0 deletions detox/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"lodash": "^4.17.5",
"minimist": "^1.2.0",
"proper-lockfile": "^3.0.2",
"sanitize-filename": "^1.6.1",
"shell-utils": "^1.0.9",
"tail": "^1.2.3",
"telnet-client": "0.15.3",
Expand Down
19 changes: 4 additions & 15 deletions detox/src/artifacts/log/android/ADBLogcatRecording.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,11 @@ class ADBLogcatRecording extends Artifact {
await this._waitUntilLogFileIsCreated;
} finally {
if (this.processPromise) {
await interruptProcess(this.processPromise);
this.processPromise = null;
const processPromise = this.processPromise;

this._waitWhileLogIsOpenedByLogcat = sleep(300).then(() => {
return retry(() => this._assertLogIsNotOpenedByApps());
});
this._waitWhileLogIsOpenedByLogcat = sleep(300)
.then(() => interruptProcess(processPromise))
.then(() => { this.processPromise = null; });
}
}
}
Expand All @@ -78,16 +77,6 @@ class ADBLogcatRecording extends Artifact {
});
}
}

async _assertLogIsNotOpenedByApps() {
const isFileOpen = await this.adb.isFileOpen(this.deviceId, this.pathToLogOnDevice);

if (isFileOpen) {
throw new DetoxRuntimeError({
message: `The log is still being opened on device (${this.deviceId}) at path: ${this.pathToLogOnDevice}`,
});
}
}
}

module.exports = ADBLogcatRecording;
140 changes: 78 additions & 62 deletions detox/src/devices/android/ADB.js
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
const _ = require('lodash');
const path = require('path');
const {execWithRetriesAndLogs, spawnAndLog} = require('../../utils/exec');
const pipeCommands = require('../../utils/pipeCommands');
const {escape} = require('../../utils/pipeCommands');
const DetoxRuntimeError = require('../../errors/DetoxRuntimeError');
const EmulatorTelnet = require('./EmulatorTelnet');
const Environment = require('../../utils/environment');

class ADB {
constructor() {
this._cachedApiLevels = new Map();
this.adbBin = path.join(Environment.getAndroidSDKPath(), 'platform-tools', 'adb');
}

async devices() {
const output = (await this.adbCmd('', 'devices')).stdout;
return await this.parseAdbDevicesConsoleOutput(output);
return this.parseAdbDevicesConsoleOutput(output);
}

async unlockScreen(deviceId) {
Expand All @@ -31,8 +33,7 @@ class ADB {
}

async _getPowerStatus(deviceId) {
const grep = pipeCommands.search.regexp;
const stdout = await this.shell(deviceId, `dumpsys power | ${grep('^[ ]*m[UW].*=')}`);
const stdout = await this.shell(deviceId, `dumpsys power | grep "^[ ]*m[UW].*="`);

return stdout
.split('\n')
Expand Down Expand Up @@ -89,15 +90,33 @@ class ADB {
}

async now(deviceId) {
return this.shell(deviceId, `date "+\\"%Y-%m-%d %T.000\\""`);
return this.shell(deviceId, `date +"%m-%d %T.000"`);
}

async isPackageInstalled(deviceId, packageId) {
const output = await this.shell(deviceId, `pm list packages ${packageId}`);
const packageRegexp = new RegExp(`^package:${escape.inQuotedRegexp(packageId)}$`, 'm');
const isInstalled = packageRegexp.test(output);

return isInstalled;
}

async install(deviceId, apkPath) {
const apiLvl = await this.apiLevel(deviceId);

let childProcess;
if (apiLvl >= 24) {
await this.adbCmd(deviceId, `install -r -g ${apkPath}`);
childProcess = await this.adbCmd(deviceId, `install -r -g ${apkPath}`);
} else {
await this.adbCmd(deviceId, `install -rg ${apkPath}`);
childProcess = await this.adbCmd(deviceId, `install -rg ${apkPath}`);
}

const [failure] = (childProcess.stdout || '').match(/^Failure \[.*\]$/m) || [];
if (failure) {
throw new DetoxRuntimeError({
message: `Failed to install app on ${deviceId}: ${apkPath}`,
debugInfo: failure,
});
}
}

Expand All @@ -110,23 +129,18 @@ class ADB {
}

async pidof(deviceId, bundleId) {
const bundleIdRegex = pipeCommands.escape.inQuotedRegexp(bundleId) + '[ ]*$';
const grep = pipeCommands.search.regexp;
const bundleIdRegex = escape.inQuotedRegexp(bundleId) + '$';

const processes = await this.shell(deviceId, `ps | ${grep(bundleIdRegex)}`).catch(() => '');
const processes = await this.shell(deviceId, `ps | grep "${bundleIdRegex}"`).catch(() => '');
if (!processes) {
return NaN;
}

return parseInt(processes.split(' ').filter(Boolean)[1], 10);
}

async shell(deviceId, cmd, options) {
return (await this.adbCmd(deviceId, `shell ${cmd}`, options)).stdout.trim();
}

async getFileSize(deviceId, filename) {
const { stdout, stderr } = await this.adbCmd(deviceId, 'shell wc -c ' + filename).catch(e => e);
const { stdout, stderr } = await this.adbCmd(deviceId, 'shell du ' + filename).catch(e => e);

if (stderr.includes('No such file or directory')) {
return -1;
Expand All @@ -135,11 +149,6 @@ class ADB {
return Number(stdout.slice(0, stdout.indexOf(' ')));
}

async isFileOpen(deviceId, filename) {
const openedByProcesses = await this.shell(deviceId, 'lsof ' + filename);
return openedByProcesses.length > 0;
}

async isBootComplete(deviceId) {
try {
const bootComplete = await this.shell(deviceId, `getprop dev.bootcomplete`);
Expand All @@ -150,12 +159,18 @@ class ADB {
}

async apiLevel(deviceId) {
const lvl = await this.shell(deviceId, `getprop ro.build.version.sdk`);
return Number(lvl);
if (this._cachedApiLevels.has(deviceId)) {
return this._cachedApiLevels.get(deviceId);
}

const lvl = Number(await this.shell(deviceId, `getprop ro.build.version.sdk`));
this._cachedApiLevels.set(deviceId, lvl);

return lvl;
}

async screencap(deviceId, path) {
return this.adbCmd(deviceId, `shell screencap ${path}`);
await this.shell(deviceId, `screencap ${path}`);
}

/***
Expand Down Expand Up @@ -185,37 +200,57 @@ class ADB {
/***
* @returns ChildProcessPromise
*/
logcat(deviceId, { expression, file, pid, time }) {
const logcatArgs = [];

if (expression) {
logcatArgs.push('-e');
logcatArgs.push(expression);
}

if (file) {
logcatArgs.push('-f');
logcatArgs.push(file);
logcat(deviceId, { file, pid, time }) {
let shellCommand = 'logcat -v brief';

// HACK: cannot make this function async, otherwise ChildProcessPromise.childProcess field will get lost,
// and this will break interruptProcess() call for any logcat promise.
const apiLevel = this._cachedApiLevels.get(deviceId);
if (time && apiLevel >= 21) {
shellCommand += ` -T "${time}"`;
}

if (pid > 0) {
logcatArgs.push(`--pid=${pid}`);
}
const __pid = String(pid).padStart(5);
shellCommand += ` | grep "(${__pid}):"`;
}

if (time) {
logcatArgs.push('-T');
logcatArgs.push(time);
if (file) {
shellCommand += ` >> ${file}`;
}

return this.spawn(deviceId, ['logcat', ...logcatArgs]);
return this.spawn(deviceId, ['shell', shellCommand]);
}

async pull(deviceId, src, dst = '') {
return this.adbCmd(deviceId, `pull "${src}" "${dst}"`);
await this.adbCmd(deviceId, `pull "${src}" "${dst}"`);
}

async rm(deviceId, path, force = false) {
return this.adbCmd(deviceId, `shell rm ${force ? '-f' : ''} "${path}"`);
await this.shell(deviceId, `rm ${force ? '-f' : ''} "${path}"`);
}

async listInstrumentation(deviceId) {
return this.shell(deviceId, 'pm list instrumentation');
}

async getInstrumentationRunner(deviceId, bundleId) {
const instrumentationRunners = await this.listInstrumentation(deviceId);
const instrumentationRunner = this._instrumentationRunnerForBundleId(instrumentationRunners, bundleId);
if (instrumentationRunner === 'undefined') {
throw new Error(`No instrumentation runner found on device ${deviceId} for package ${bundleId}`);
}

return instrumentationRunner;
}

_instrumentationRunnerForBundleId(instrumentationRunners, bundleId) {
const runnerForBundleRegEx = new RegExp(`^instrumentation:(.*) \\(target=${bundleId.replace(new RegExp('\\.', 'g'), "\\.")}\\)$`, 'gm');
return _.get(runnerForBundleRegEx.exec(instrumentationRunners), [1], 'undefined');
}

async shell(deviceId, cmd, options) {
return (await this.adbCmd(deviceId, `shell "${escape.inQuotedString(cmd)}"`, options)).stdout.trim();
}

async adbCmd(deviceId, params, options) {
Expand All @@ -224,7 +259,7 @@ class ADB {
const retries = _.get(options, 'retries', 1);
_.unset(options, 'retries');

return await execWithRetriesAndLogs(cmd, options, undefined, retries);
return execWithRetriesAndLogs(cmd, options, undefined, retries);
}

/***
Expand All @@ -234,25 +269,6 @@ class ADB {
const serial = deviceId ? ['-s', deviceId] : [];
return spawnAndLog(this.adbBin, [...serial, ...params]);
}

async listInstrumentation(deviceId) {
return await this.shell(deviceId, 'pm list instrumentation');
}

instrumentationRunnerForBundleId(instrumentationRunners, bundleId) {
const runnerForBundleRegEx = new RegExp(`^instrumentation:(.*) \\(target=${bundleId.replace(new RegExp('\\.', 'g'), "\\.")}\\)$`, 'gm');
return _.get(runnerForBundleRegEx.exec(instrumentationRunners), [1], 'undefined');
}

async getInstrumentationRunner(deviceId, bundleId) {
const instrumentationRunners = await this.listInstrumentation(deviceId);
const instrumentationRunner = this.instrumentationRunnerForBundleId(instrumentationRunners, bundleId);
if (instrumentationRunner === 'undefined') {
throw new Error(`No instrumentation runner found on device ${deviceId} for package ${bundleId}`);
}

return instrumentationRunner;
}
}

module.exports = ADB;
30 changes: 12 additions & 18 deletions detox/src/devices/android/ADB.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,22 @@ describe('ADB', () => {
});

it(`pidof (success)`, async () => {
adb.shell = async () => `u0_a19 2199 1701 3554600 70264 0 0 s com.google.android.ext.services `;
jest.spyOn(adb, 'shell').mockImplementation(async () =>
`u0_a19 2199 1701 3554600 70264 0 0 s com.google.android.ext.services `);

expect(await adb.pidof('', 'com.google.android.ext.services')).toBe(2199);
});

it(`pidof (failure)`, async () => {
adb.shell = async () => ``;
jest.spyOn(adb, 'shell').mockImplementation(async () => '');
expect(await adb.pidof('', 'com.google.android.ext.services')).toBe(NaN);
});

describe('unlockScreen', () => {
const deviceId = 'mockEmulator';

async function unlockScreenWithPowerStatus(mWakefulness, mUserActivityTimeoutOverrideFromWindowManager) {
adb.shell = jest.fn().mockReturnValue(`
jest.spyOn(adb, 'shell').mockImplementation(async () => `
mWakefulness=${mWakefulness}
mWakefulnessChanging=false
mWakeLockSummary=0x0
Expand Down Expand Up @@ -116,11 +118,11 @@ describe('ADB', () => {

it(`listInstrumentation passes the right deviceId`, async () => {
const deviceId = 'aDeviceId';
const spyShell = jest.spyOn(adb, 'shell');
jest.spyOn(adb, 'shell');

await adb.listInstrumentation(deviceId);

expect(spyShell).toBeCalledWith(deviceId, expect.any(String));
expect(adb.shell).toBeCalledWith(deviceId, 'pm list instrumentation');
});

it(`Parse 'adb device' output`, async () => {
Expand All @@ -144,18 +146,7 @@ describe('ADB', () => {
expect(actual).toEqual(parsedDevices);
});

it(`getInstrumentationRunner passes the right deviceId`, async () => {
const deviceId = 'aDeviceId';
const spyRunnerForBundle = jest.spyOn(adb, 'instrumentationRunnerForBundleId');
spyRunnerForBundle.mockReturnValue('');
const spyShell = jest.spyOn(adb, 'shell');

await adb.getInstrumentationRunner(deviceId, 'com.whatever.package');

expect(spyShell).toBeCalledWith(deviceId, expect.any(String));
});

it(`instrumentationRunnerForBundleId parses the correct runner for the package`, async () => {
it(`getInstrumentationRunner parses the correct runner for the package`, async () => {
const expectedRunner = "com.example.android.apis/.app.LocalSampleInstrumentation";
const expectedPackage = "com.example.android.apis";
const instrumentationRunnersShellOutput =
Expand All @@ -164,8 +155,11 @@ describe('ADB', () => {
`instrumentation:${expectedRunner} (target=${expectedPackage})\n` +
"instrumentation:org.chromium.webview_shell/.WebViewLayoutTestRunner (target=org.chromium.webview_shell)\n";

const result = await adb.instrumentationRunnerForBundleId(instrumentationRunnersShellOutput, expectedPackage);
jest.spyOn(adb, 'shell').mockImplementation(async () => instrumentationRunnersShellOutput);

const result = await adb.getInstrumentationRunner('aDeviceId', expectedPackage);

expect(adb.shell).toBeCalledWith('aDeviceId', 'pm list instrumentation');
expect(result).toEqual(expectedRunner);
});
});
Expand Down
Loading

0 comments on commit 3e11f66

Please sign in to comment.