Skip to content

Commit

Permalink
Merge pull request #4543 from wix/device_allocation_retry
Browse files Browse the repository at this point in the history
fix(Android): Added retry for Genymotion device allocation
  • Loading branch information
gosha212 authored Aug 5, 2024
2 parents 619509a + 5d1dc9d commit 5729751
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 1 deletion.
7 changes: 7 additions & 0 deletions detox/src/devices/allocation/DeviceAllocator.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ class DeviceAllocator {
this._driver.emergencyCleanup();
}
}

isRecoverableError(error) {
if (typeof this._driver.isRecoverableError !== 'function') {
return false;
}
return this._driver.isRecoverableError(error);
}
}

module.exports = DeviceAllocator;
13 changes: 13 additions & 0 deletions detox/src/devices/allocation/DeviceAllocator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,17 @@ describe('Device allocator', () => {
expect(allocDriver.emergencyCleanup).toHaveBeenCalled();
});
});

describe('#isRecoverableError()', function() {
test('should return false if the driver does not implement this function', () => {
delete allocDriver.isRecoverableError;
expect(deviceAllocator.isRecoverableError(new Error())).toBe(false);
});

test('should return the driver\'s isRecoverableError result', () => {
allocDriver.isRecoverableError = jest.fn().mockReturnValue(true);
expect(deviceAllocator.isRecoverableError(new Error())).toBe(true);
expect(allocDriver.isRecoverableError).toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ export interface AllocationDriverBase {
free(cookie: DeviceCookie, options: DeallocOptions): Promise<void>;
cleanup?(): Promise<void>;
emergencyCleanup?(): void;
isRecoverableError?(error: any): boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,17 @@ class GenyAllocDriver {
this._reportGlobalCleanupSummary(deletionLeaks);
}

/**
* The current error we could recover from in the context of Genymotion Cloud is when the device is not found.
* The error message will contain the following text adb: device 'localhost:xxxxx' not found
* @param error
* @returns {boolean}
*/
isRecoverableError(error) {
const errorStr = JSON.stringify(error);
return errorStr.indexOf('adb: device \'localhost:') !== -1;
}

emergencyCleanup() {
const instances = this._genyRegistry.getInstances();
this._reportGlobalCleanupSummary(instances);
Expand Down
21 changes: 20 additions & 1 deletion detox/src/realms/DetoxPrimaryContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const temporary = require('../artifacts/utils/temporaryPath');
const { DetoxRuntimeError } = require('../errors');
const SessionState = require('../ipc/SessionState');
const { getCurrentCommand } = require('../utils/argparse');
const retry = require('../utils/retry');
const uuid = require('../utils/uuid');

const DetoxContext = require('./DetoxContext');
Expand All @@ -27,6 +28,7 @@ const _cookieAllocators = Symbol('cookieAllocators');
const _deviceAllocators = Symbol('deviceAllocators');
const _createDeviceAllocator = Symbol('createDeviceAllocator');
const _createDeviceAllocatorInstance = Symbol('createDeviceAllocatorInstance');
const _allocateDeviceOnce = Symbol('allocateDeviceOnce');
//#endregion

class DetoxPrimaryContext extends DetoxContext {
Expand Down Expand Up @@ -154,6 +156,20 @@ class DetoxPrimaryContext extends DetoxContext {
/** @override */
async [symbols.allocateDevice](deviceConfig) {
const deviceAllocator = await this[_createDeviceAllocator](deviceConfig);

const retryOptions = {
backoff: 'none',
retries: 5,
interval: 25000,
conditionFn: (e) => deviceAllocator.isRecoverableError(e),
};

return await retry(retryOptions, async () => {
return await this[_allocateDeviceOnce](deviceAllocator, deviceConfig);
});
}

async [_allocateDeviceOnce](deviceAllocator, deviceConfig) {
const deviceCookie = await deviceAllocator.allocate(deviceConfig);
this[_cookieAllocators][deviceCookie.id] = deviceAllocator;

Expand All @@ -163,7 +179,10 @@ class DetoxPrimaryContext extends DetoxContext {
try {
await deviceAllocator.free(deviceCookie, { shutdown: true });
} catch (e2) {
this[symbols.logger].error({ cat: 'device', err: e2 }, `Failed to free ${deviceCookie.name || deviceCookie.id} after a failed allocation attempt`);
this[symbols.logger].error({
cat: 'device',
err: e2
}, `Failed to free ${deviceCookie.name || deviceCookie.id} after a failed allocation attempt`);
} finally {
delete this[_cookieAllocators][deviceCookie.id];
}
Expand Down
7 changes: 7 additions & 0 deletions detox/src/realms/DetoxPrimaryContext.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ describe('DetoxPrimaryContext', () => {
let facade;
/** @type {import('./symbols')} */
let symbols;
/** @type {jest.Mock<import('../utils/retry')>} */
let retry;

const detoxServer = () => latestInstanceOf(DetoxServer);
const ipcServer = () => latestInstanceOf(IPCServer);
Expand Down Expand Up @@ -466,6 +468,7 @@ describe('DetoxPrimaryContext', () => {
free: jest.fn(),
cleanup: jest.fn(),
emergencyCleanup: jest.fn(),
isRecoverableError: jest.fn().mockReturnValue(true)
};

jest.mock('../environmentFactory');
Expand All @@ -481,6 +484,10 @@ describe('DetoxPrimaryContext', () => {

jest.mock('../DetoxWorker');
DetoxWorker = jest.requireMock('../DetoxWorker');

jest.mock('../utils/retry');
retry = jest.requireMock('../utils/retry');
retry.mockImplementation((opts, callback) => callback());
}

async function allocateSomeDevice() {
Expand Down

0 comments on commit 5729751

Please sign in to comment.