Fix: ADB.pidof(deviceId, bundleId) does not work consistently across…

… Android versions (#839) * Resolves #831 * feat: makes launch() implementation for Android stricter - it fails if PID is not found
wix · Jul 19, 2018 · ac4ea8d · ac4ea8d
1 parent 0d4ae31
commit ac4ea8d
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 24 deletions.
diff --git a/detox/src/devices/AndroidDriver.js b/detox/src/devices/AndroidDriver.js
@@ -16,6 +16,7 @@ const ADBLogcatPlugin = require('../artifacts/log/android/ADBLogcatPlugin');
 const ADBScreencapPlugin = require('../artifacts/screenshot/ADBScreencapPlugin');
 const ADBScreenrecorderPlugin = require('../artifacts/video/ADBScreenrecorderPlugin');
 const AndroidDevicePathBuilder = require('../artifacts/utils/AndroidDevicePathBuilder');
+const DetoxRuntimeError = require('../errors/DetoxRuntimeError');
 const sleep = require('../utils/sleep');
 
 const EspressoDetox = 'com.wix.detox.espresso.EspressoDetox';
@@ -109,7 +110,17 @@ class AndroidDriver extends DeviceDriverBase {
       this.terminateInstrumentation();
     });
 
-    return this._queryPID(deviceId, bundleId);
+    const appPID = await this._queryPID(deviceId, bundleId);
+    if (isNaN(appPID)) {
+      log.warn(await this.adb.shell(deviceId, 'ps'));
+
+      throw new DetoxRuntimeError({
+        message: `Failed to find PID of the launched bundle: ${bundleId}`,
+        hint: `You might want to check "adb logcat" logs - maybe the app has crashed.`,
+      });
+    }
+
+    return appPID;
   }
 
   async _queryPID(deviceId, bundleId, waitAtStart = true) {

diff --git a/detox/src/devices/android/ADB.js b/detox/src/devices/android/ADB.js
@@ -2,9 +2,9 @@ const _ = require('lodash');
 const child_process = require('child_process');
 const path = require('path');
 const {execWithRetriesAndLogs, spawnAndLog} = require('../../utils/exec');
+const regexEscape = require('../../utils/regexEscape');
 const EmulatorTelnet = require('./EmulatorTelnet');
 const Environment = require('../../utils/environment');
-
 class ADB {
 
   constructor() {
@@ -71,21 +71,12 @@ class ADB {
   }
 
   async pidof(deviceId, bundleId) {
-    const processes = await this.shell(deviceId, `ps -AMo NAME,PID`);
-    const bundleIndex = processes.indexOf(bundleId + ' ');
-
-    if (bundleIndex === -1) {
+    const processes = await this.shell(deviceId, `ps | grep "${regexEscape(bundleId)}\\s*$"`).catch(() => '');
+    if (!processes) {
       return NaN;
     }
 
-    const pidStart = bundleIndex + bundleId.length + 1;
-    const pidEnd = processes.indexOf('\n', pidStart);
-
-    const pidString = (pidEnd === -1)
-      ? processes.slice(pidStart)
-      : processes.slice(pidStart, pidEnd);
-
-    return parseInt(pidString, 10);
+    return parseInt(processes.split(' ').filter(Boolean)[1], 10);
   }
 
   async shell(deviceId, cmd) {

diff --git a/detox/src/devices/android/ADB.test.js b/detox/src/devices/android/ADB.test.js
@@ -45,16 +45,14 @@ describe('ADB', () => {
     expect(exec).toHaveBeenCalledTimes(1);
   });
 
-  it(`pidof`, async () => {
-    adb.shell = async () => `
-com.google.android.apps.google.google.google.test.go 2245
-com.wix.detox.test                                  10670
-com.google.android.dialer                           17214`;
-
-    expect(await adb.pidof('', 'com.google.android.apps.google.google.google.test.go')).toBe(2245);
-    expect(await adb.pidof('', 'com.wix.detox.test')).toBe(10670);
-    expect(await adb.pidof('', 'com.google.android.dialer')).toBe(17214);
-    expect(await adb.pidof('', 'unexisting bundle')).toBe(NaN);
+  it(`pidof (success)`, async () => {
+    adb.shell = 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 () => ``;
+    expect(await adb.pidof('', 'com.google.android.ext.services')).toBe(NaN);
   });
 
   it(`unlockScreen`, async () => {

diff --git a/detox/src/utils/regexEscape.js b/detox/src/utils/regexEscape.js
@@ -0,0 +1,7 @@
+const SPECIAL_CHARS = /([\^\$\[\]\*\.\\])/g;
+
+function regexEscape(exactString) {
+  return exactString.replace(SPECIAL_CHARS, "\\$1");
+}
+
+module.exports = regexEscape;
diff --git a/detox/src/utils/regexEscape.test.js b/detox/src/utils/regexEscape.test.js
@@ -0,0 +1,23 @@
+const regexEscape = require('./regexEscape');
+
+describe(regexEscape.name, () => {
+  it('should not escape non-special characters', () => {
+    const nonspecial = 'bundle_name';
+    expect(regexEscape(nonspecial)).toBe(nonspecial);
+  });
+
+  it('should escape [\\]', () => {
+    const bundleId = '[kworker\\0:0]';
+    expect(regexEscape(bundleId)).toBe('\\[kworker\\\\0:0\\]');
+  });
+
+  it('should escape ^*$', () => {
+    const bundleId = '^ma*tch$';
+    expect(regexEscape(bundleId)).toBe('\\^ma\\*tch\\$');
+  });
+
+  it('should escape dots', () => {
+    const bundleId = 'com.company.bundle';
+    expect(regexEscape(bundleId)).toBe('com\\.company\\.bundle');
+  });
+});