Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Android): Added retry for Genymotion device allocation #4543

Merged
merged 3 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading