fix: ADB.pidof integration issue with grep and CRLF edge case

detox/package.json

@@ -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",

detox/src/artifacts/log/android/ADBLogcatRecording.js

@@ -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; });
       }
     }
   }
@@ -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;

detox/src/devices/android/ADB.js

@@ -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) {
@@ -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')
@@ -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,
+      });
     }
   }
 
@@ -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;
@@ -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`);
@@ -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}`);
   }
 
   /***
@@ -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) {
@@ -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);
   }
 
   /***
@@ -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;

detox/src/devices/android/ADB.test.js

@@ -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
@@ -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 () => {
@@ -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 =
@@ -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);
   });
 });