wip: Allocation for emulator reuse

d4vidi · d4vidi · commit 3dae97a6adc4 · 2026-01-07T20:18:05.000+02:00
diff --git a/detox/src/devices/allocation/drivers/android/FreeDeviceFinder.js b/detox/src/devices/allocation/drivers/android/FreeDeviceFinder.js
@@ -12,12 +12,15 @@ class FreeDeviceFinder {
     this.deviceRegistry = deviceRegistry;
   }
 
+  /**
+   * @returns {Promise<import('../../../common/drivers/android/tools/DeviceHandle')>}
+   */
   async findFreeDevice(deviceQuery) {
     const { devices } = await this.adb.devices();
     const takenDevices = this.deviceRegistry.getTakenDevicesSync();
     for (const candidate of devices) {
       if (await this._isDeviceFreeAndMatching(takenDevices, candidate, deviceQuery)) {
-        return candidate.adbName;
+        return candidate;
       }
     }
     return null;
diff --git a/detox/src/devices/allocation/drivers/android/attached/AttachedAndroidAllocDriver.js b/detox/src/devices/allocation/drivers/android/attached/AttachedAndroidAllocDriver.js
@@ -27,7 +27,8 @@ class AttachedAndroidAllocDriver extends AndroidAllocDriver {
    */
   async allocate(deviceConfig) {
     const adbNamePattern = deviceConfig.device.adbName;
-    const adbName = await this._deviceRegistry.registerDevice(() => this._freeDeviceFinder.findFreeDevice(adbNamePattern));
+    const adbName = await this._deviceRegistry.registerDevice(async () =>
+      (await this._freeDeviceFinder.findFreeDevice(adbNamePattern)).adbName);
 
     return { id: adbName, adbName };
   }
diff --git a/detox/src/devices/allocation/drivers/android/emulator/EmulatorAllocDriver.js b/detox/src/devices/allocation/drivers/android/emulator/EmulatorAllocDriver.js
@@ -63,15 +63,18 @@ class EmulatorAllocDriver extends AndroidAllocDriver {
     let adbName;
 
     await this._deviceRegistry.registerDevice(async () => {
-      adbName = await this._freeDeviceFinder.findFreeDevice(avdName);
+      const device = await this._freeDeviceFinder.findFreeDevice(avdName);
 
-      if (adbName) {
-        adbServerPort = adbPortRegistry.getPort(adbName);
+      if (device) {
+        adbName = device.adbName;
+        adbServerPort = device.adbServerPort;
+        adbPortRegistry.register(adbName, adbServerPort);
       } else {
         const port = await this._freePortFinder.findFreePort();
 
         adbName = `emulator-${port}`;
         adbServerPort = this._nextAdbServerPort++;
+        adbPortRegistry.register(adbName, adbServerPort);
 
         await this._emulatorLauncher.launch({
           bootArgs: deviceConfig.bootArgs,
diff --git a/detox/src/devices/allocation/drivers/android/emulator/FreePortFinder.js b/detox/src/devices/allocation/drivers/android/emulator/FreePortFinder.js
@@ -1,4 +1,4 @@
-const net = require('net');
+const { isPortTaken } = require('../../../../../utils/portUtils');
 
 class FreePortFinder {
   constructor({ min = 10000, max = 20000 } = {}) {
@@ -14,24 +14,10 @@ class FreePortFinder {
       const max = this._max;
       port = Math.random() * (max - min) + min;
       port = port & 0xFFFFFFFE; // Should always be even
-    } while (await this.isPortTaken(port));
+    } while (await isPortTaken(port));
 
     return port;
   }
-
-  async isPortTaken(port) {
-    return new Promise((resolve, reject) => {
-      const tester = net.createServer()
-        .once('error', /** @param {*} err */ (err) => {
-          /* istanbul ignore next */
-          return err && err.code === 'EADDRINUSE' ? resolve(true) : reject(err);
-        })
-        .once('listening', () => {
-          tester.once('close', () => resolve(false)).close();
-        })
-        .listen(port);
-    });
-  }
 }
 
 module.exports = FreePortFinder;
diff --git a/detox/src/devices/allocation/drivers/android/emulator/FreePortFinder.test.js b/detox/src/devices/allocation/drivers/android/emulator/FreePortFinder.test.js
@@ -1,39 +1,21 @@
-const net = require('net');
-
-const FreePortFinder = require('./FreePortFinder');
-
 describe('FreePortFinder', () => {
   let finder;
-  let server;
+  let isPortTaken;
 
   beforeEach(() => {
-    finder = new FreePortFinder();
-  });
+    jest.mock('../../../../../utils/portUtils');
+    isPortTaken = require('../../../../../utils/portUtils').isPortTaken;
+    isPortTaken.mockResolvedValue(false);
 
-  afterEach(done => {
-    if (server) {
-      server.close(done);
-    } else {
-      done();
-    }
+    const FreePortFinder = require('./FreePortFinder');
+    finder = new FreePortFinder();
   });
 
   test('should find a free port', async () => {
     const port = await finder.findFreePort();
     expect(port).toBeGreaterThanOrEqual(10000);
     expect(port).toBeLessThanOrEqual(20000);
     expect(port % 2).toBe(0);
-    await expect(finder.isPortTaken(port)).resolves.toBe(false);
-  });
-
-  test('should identify a taken port', async () => {
-    server = net.createServer();
-    const portTaken = await new Promise(resolve => {
-      server.listen(0, () => { // 0 means random available port
-        resolve(server.address().port);
-      });
-    });
-
-    await expect(finder.isPortTaken(portTaken)).resolves.toBe(true);
+    expect(isPortTaken).toHaveBeenCalled();
   });
 });
diff --git a/detox/src/devices/common/drivers/android/AdbPortRegistry.js b/detox/src/devices/common/drivers/android/AdbPortRegistry.js
@@ -18,13 +18,6 @@ class AdbPortRegistry {
   getPort(adbName) {
     return this._registry.get(adbName);
   }
-
-  /**
-   * @returns { number[] }
-   */
-  getAllPorts() {
-    return this._registry.values().toArray();
-  }
 }
 
 module.exports = new AdbPortRegistry();
diff --git a/detox/src/devices/common/drivers/android/exec/ADB.js b/detox/src/devices/common/drivers/android/exec/ADB.js
@@ -8,6 +8,7 @@ const { escape } = require('../../../../../utils/pipeCommands');
 const adbPortRegistry = require('../AdbPortRegistry');
 const DeviceHandle = require('../tools/DeviceHandle');
 const EmulatorHandle = require('../tools/EmulatorHandle');
+const { isPortTaken } = require('../../../../../utils/portUtils');
 
 const DEFAULT_EXEC_OPTIONS = {
   retries: 1,
@@ -36,28 +37,29 @@ class ADB {
   }
 
   async devices(options) {
-    let devices = [];
+    const devicesByPort = {};
     const stdouts = [];
-    const ports = [ADB_SERVER_BASE_PORT, ...adbPortRegistry.getAllPorts()];
-
-    for (const port of ports) {
+    for (let port = ADB_SERVER_BASE_PORT; await isPortTaken(port); port++) {
       const { stdout } = await this.adbCmdWithPort('', port, 'devices', { verbosity: 'high', ...options });
-      devices.push(
-        _.chain(stdout)
-          .trim()
-          .split('\n')
-          .slice(1)
-          .map(s => _.trim(s))
-          .value()
-      );
+      devicesByPort[port] = _.chain(stdout)
+        .trim()
+        .split('\n')
+        .slice(1)
+        .map(s => _.trim(s))
+        .value();
       stdouts.push(stdout);
     }
 
+    const devices =
+      _.flatMap(Object.keys(devicesByPort), port =>
+        _.map(devicesByPort[port],
+            s => s.startsWith('emulator-')
+              ? new EmulatorHandle(s, Number(port))
+              : new DeviceHandle(s, Number(port))
+        ));
+
     return {
-      devices: _.flatten(devices)
-        .map(s => s.startsWith('emulator-')
-          ? new EmulatorHandle(s)
-          : new DeviceHandle(s)),
+      devices,
       stdout: stdouts.join('\n'),
     };
   }
diff --git a/detox/src/devices/common/drivers/android/exec/ADB.test.js b/detox/src/devices/common/drivers/android/exec/ADB.test.js
@@ -2,6 +2,7 @@
 describe('ADB', () => {
   const deviceId = 'mockEmulator';
   const adbBinPath = `/Android/sdk-mock/platform-tools/adb`;
+  const baseAdbServerPort = 5037;
 
   let ADB;
   let adb;
@@ -11,6 +12,7 @@ describe('ADB', () => {
   let spawnAndLog;
   let spawnWithRetriesAndLogs;
   let adbPortRegistry;
+  let isPortTaken;
 
   beforeEach(() => {
     jest.mock('../../../../../utils/logger');
@@ -38,14 +40,16 @@ describe('ADB', () => {
 
     jest.mock('../AdbPortRegistry');
     adbPortRegistry = require('../AdbPortRegistry');
-    adbPortRegistry.getAllPorts.mockReturnValue([]);
+
+    jest.mock('../../../../../utils/portUtils');
+    isPortTaken = require('../../../../../utils/portUtils').isPortTaken;
+    isPortTaken.mockResolvedValueOnce(true);
 
     ADB = require('./ADB');
     adb = new ADB();
   });
 
   describe('devices', () => {
-
     const mockDeviceEntry = 'MOCK_SERIAL\tdevice';
     const wifiDeviceEntry = '192.168.60.101:6666\tdevice';
     const emulatorEntry = 'emulator-5554\tdevice';
@@ -78,10 +82,10 @@ describe('ADB', () => {
         EmulatorHandle.mock.instances[0],
         EmulatorHandle.mock.instances[1],
       ]);
-      expect(DeviceHandle).toHaveBeenCalledWith(mockDevices[0]);
-      expect(DeviceHandle).toHaveBeenCalledWith(mockDevices[1]);
-      expect(EmulatorHandle).toHaveBeenCalledWith(mockDevices[2]);
-      expect(EmulatorHandle).toHaveBeenCalledWith(mockDevices[3]);
+      expect(DeviceHandle).toHaveBeenCalledWith(mockDevices[0], baseAdbServerPort);
+      expect(DeviceHandle).toHaveBeenCalledWith(mockDevices[1], baseAdbServerPort);
+      expect(EmulatorHandle).toHaveBeenCalledWith(mockDevices[2], baseAdbServerPort);
+      expect(EmulatorHandle).toHaveBeenCalledWith(mockDevices[3], baseAdbServerPort);
     });
 
     it(`should return an empty list if no devices are available`, async () => {
@@ -103,18 +107,17 @@ describe('ADB', () => {
       expect(execWithRetriesAndLogs).toHaveBeenCalledWith(expect.any(String), options);
     });
 
-    it('should process devices correctly when adbPortRegistry.getAllPorts() returns a port array only on the 2nd call', async () => {
-      const regPort = 5038;
-      adbPortRegistry.getAllPorts.mockReturnValue([regPort]);
+    it('should process devices correctly when emulators are on another adb-server', async () => {
+      isPortTaken.mockImplementation((port) => (port === baseAdbServerPort || port === baseAdbServerPort + 1));
       execWithRetriesAndLogs
         .mockReturnValueOnce({ stdout: adbDevicesStdout([emulatorEntry]) }) // Result for the standard port (5037)
         .mockReturnValueOnce({ stdout: adbDevicesStdout([emulator5556Entry]) }); // Result for the port from the adb-ports registry
 
       const { devices } = await adb.devices();
 
       expect(devices).toHaveLength(2);
-      expect(EmulatorHandle).toHaveBeenCalledWith(emulatorEntry);
-      expect(EmulatorHandle).toHaveBeenCalledWith(emulator5556Entry);
+      expect(EmulatorHandle).toHaveBeenCalledWith(emulatorEntry, baseAdbServerPort);
+      expect(EmulatorHandle).toHaveBeenCalledWith(emulator5556Entry, baseAdbServerPort + 1);
     });
   });
 
diff --git a/detox/src/devices/common/drivers/android/tools/DeviceHandle.js b/detox/src/devices/common/drivers/android/tools/DeviceHandle.js
@@ -1,9 +1,10 @@
 class DeviceHandle {
-  constructor(deviceString) {
+  constructor(deviceString, adbServerPort) {
     const [adbName, status] = deviceString.split('\t');
     this.type = this._inferDeviceType(adbName);
     this.adbName = adbName;
     this.status = status;
+    this.adbServerPort = adbServerPort;
   }
 
   _inferDeviceType(adbName) {
diff --git a/detox/src/devices/common/drivers/android/tools/EmulatorHandle.js b/detox/src/devices/common/drivers/android/tools/EmulatorHandle.js
@@ -2,8 +2,8 @@ const DeviceHandle = require('./DeviceHandle');
 const EmulatorTelnet = require('./EmulatorTelnet');
 
 class EmulatorHandle extends DeviceHandle {
-  constructor(deviceString) {
-    super(deviceString);
+  constructor(deviceString, adbServerPort) {
+    super(deviceString, adbServerPort);
     this._telnet = new EmulatorTelnet();
 
     this.port = this.adbName.split('-')[1];
diff --git a/detox/src/utils/portUtils.js b/detox/src/utils/portUtils.js
@@ -0,0 +1,56 @@
+const net = require('net');
+
+/**
+ * Checks if a given port is currently taken (in use)
+ * Uses a two-step approach:
+ * 1. First tries to connect to the port (detects services like ADB that are listening)
+ * 2. If connection fails, tries to bind to the port (detects if port is in use)
+ * @param {number} port - The port number to check
+ * @returns {Promise<boolean>} - Resolves to true if the port is taken, false if it's available
+ */
+async function isPortTaken(port) {
+  return new Promise((resolve, reject) => {
+    function tryBind() {
+      const tester = net.createServer()
+        .once('error', /** @param {*} err */ (err) => {
+          /* istanbul ignore next */
+          return err && err.code === 'EADDRINUSE' ? resolve(true) : reject(err);
+        })
+        .once('listening', () => {
+          tester.once('close', () => resolve(false)).close();
+        })
+        .listen(port);
+    }
+
+    // Try to connect to the port to detect if something is listening (e.g., ADB server)
+    const socket = new net.Socket();
+    const timeout = setTimeout(() => {
+      socket.destroy();
+      // Connection timeout means port might be free, try binding instead
+      tryBind();
+    }, 100);
+
+    socket.once('connect', () => {
+      clearTimeout(timeout);
+      socket.destroy();
+      resolve(true);
+    });
+
+    socket.once('error', /** @param {NodeJS.ErrnoException} err */ (err) => {
+      clearTimeout(timeout);
+      if (err.code === 'ECONNREFUSED') {
+        // Connection refused means nothing is listening, try binding to confirm
+        tryBind();
+      } else {
+        // Other errors might indicate port is in use, try binding to confirm
+        tryBind();
+      }
+    });
+
+    socket.connect(port, 'localhost');
+  });
+}
+
+module.exports = {
+  isPortTaken,
+};
diff --git a/detox/src/utils/portUtils.test.js b/detox/src/utils/portUtils.test.js
@@ -0,0 +1,41 @@
+const net = require('net');
+
+const { isPortTaken } = require('./portUtils');
+
+describe('portUtils', () => {
+  let server;
+
+  afterEach(done => {
+    if (server) {
+      server.close(done);
+    } else {
+      done();
+    }
+  });
+
+  test('should identify a taken port', async () => {
+    server = net.createServer();
+    const portTaken = await new Promise(resolve => {
+      server.listen(0, () => {
+        resolve(server.address().port);
+      });
+    });
+
+    await expect(isPortTaken(portTaken)).resolves.toEqual(true);
+  });
+
+  test('should identify an available port', async () => {
+    server = net.createServer();
+    const availablePort = await new Promise(resolve => {
+      server.listen(0, () => {
+        const port = server.address().port;
+        server.close(() => {
+          resolve(port);
+          server = null;
+        });
+      });
+    });
+
+    await expect(isPortTaken(availablePort)).resolves.toEqual(false);
+  });
+});

Original file line number	Diff line number	Diff line change
`@@ -12,12 +12,15 @@ class FreeDeviceFinder {`
`12`	`12`	`this.deviceRegistry = deviceRegistry;`
`13`	`13`	`}`
`14`	`14`
	`15`	`+ /**`
	`16`	`+ * @returns {Promise<import('../../../common/drivers/android/tools/DeviceHandle')>}`
	`17`	`+ */`
`15`	`18`	`async findFreeDevice(deviceQuery) {`
`16`	`19`	`const { devices } = await this.adb.devices();`
`17`	`20`	`const takenDevices = this.deviceRegistry.getTakenDevicesSync();`
`18`	`21`	`for (const candidate of devices) {`
`19`	`22`	`if (await this._isDeviceFreeAndMatching(takenDevices, candidate, deviceQuery)) {`
`20`		`- return candidate.adbName;`
	`23`	`+ return candidate;`
`21`	`24`	`}`
`22`	`25`	`}`
`23`	`26`	`return null;`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,8 @@ class AttachedAndroidAllocDriver extends AndroidAllocDriver {`
`27`	`27`	`*/`
`28`	`28`	`async allocate(deviceConfig) {`
`29`	`29`	`const adbNamePattern = deviceConfig.device.adbName;`
`30`		`- const adbName = await this._deviceRegistry.registerDevice(() => this._freeDeviceFinder.findFreeDevice(adbNamePattern));`
	`30`	`+ const adbName = await this._deviceRegistry.registerDevice(async () =>`
	`31`	`+ (await this._freeDeviceFinder.findFreeDevice(adbNamePattern)).adbName);`
`31`	`32`
`32`	`33`	`return { id: adbName, adbName };`
`33`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,13 +18,6 @@ class AdbPortRegistry {`
`18`	`18`	`getPort(adbName) {`
`19`	`19`	`return this._registry.get(adbName);`
`20`	`20`	`}`
`21`		`-`
`22`		`- /**`
`23`		`- * @returns { number[] }`
`24`		`- */`
`25`		`- getAllPorts() {`
`26`		`- return this._registry.values().toArray();`
`27`		`- }`
`28`	`21`	`}`
`29`	`22`
`30`	`23`	`module.exports = new AdbPortRegistry();`