From c6680fada39caca37abaa0a116e78f26daf30a1c Mon Sep 17 00:00:00 2001 From: Leo Natan Date: Wed, 14 Mar 2018 20:16:50 +0200 Subject: [PATCH] Make createUserNotificationFile parallel safe Closes #601 --- detox/src/devices/Device.js | 33 ++++++++++-------- detox/src/devices/Device.test.js | 6 ++-- detox/src/devices/DeviceDriverBase.js | 47 +++++++++++++++++++++----- detox/src/devices/IosDriver.js | 5 ++- detox/test/e2e/k-user-notifications.js | 6 ++-- detox/test/e2e/o-urls.js | 2 +- 6 files changed, 67 insertions(+), 32 deletions(-) diff --git a/detox/src/devices/Device.js b/detox/src/devices/Device.js index b92c645dd2..4dafc6d612 100644 --- a/detox/src/devices/Device.js +++ b/detox/src/devices/Device.js @@ -44,14 +44,14 @@ class Device { async launchApp(params = {newInstance: false}, bundleId) { await this._artifactsCopier.handleAppRelaunch(); - let paramsCounter = 0; + let paramsCounter = 0; - const singleParams = ['url', 'userNotification']; - singleParams.forEach((item) => { - if(params[item]) { - paramsCounter += 1; - } - }) + const singleParams = ['url', 'userNotification']; + singleParams.forEach((item) => { + if(params[item]) { + paramsCounter += 1; + } + }) if (paramsCounter > 1) { throw new Error(`Call to 'launchApp(${JSON.stringify(params)})' must contain only one of ${JSON.stringify(singleParams)}.`); @@ -76,11 +76,11 @@ class Device { baseLaunchArgs['detoxSourceAppOverride'] = params.sourceApp; } } else if (params.userNotification) { - const notificationFilePath = this.deviceDriver.createPushNotificationJson(params.userNotification); + const notificationFilePath = this.deviceDriver.createUserNotificationFile(params.userNotification); baseLaunchArgs['detoxUserNotificationDataURL'] = notificationFilePath; - //`params` will be used later for `deliverPayload`, so remove the actual notification and add the file URL - delete params.userNotification; - params.detoxUserNotificationDataURL = notificationFilePath; + //`params` will be used later for `deliverPayload`, so remove the actual notification and add the file URL + delete params.userNotification; + params.detoxUserNotificationDataURL = notificationFilePath; } if (params.permissions) { @@ -90,14 +90,18 @@ class Device { const _bundleId = bundleId || this._bundleId; if (this._isAppInBackground(params, _bundleId)) { if (params.url || params.detoxUserNotificationDataURL) { - await this.deviceDriver.deliverPayload({...params, delayPayload: true}); - } + await this.deviceDriver.deliverPayload({...params, delayPayload: true}); + } } const processId = await this.deviceDriver.launch(this._deviceId, _bundleId, this._prepareLaunchArgs(baseLaunchArgs)); this._processes[_bundleId] = processId; await this.deviceDriver.waitUntilReady(); + + if(params.detoxUserNotificationDataURL) { + this.deviceDriver.cleanupRandomDirectory(params.detoxUserNotificationDataURL); + } } _isAppInBackground(params, _bundleId) { @@ -162,8 +166,9 @@ class Device { } async sendUserNotification(params) { - const notificationFilePath = this.deviceDriver.createPushNotificationJson(params); + const notificationFilePath = this.deviceDriver.createUserNotificationFile(params); await this.deviceDriver.deliverPayload({'detoxUserNotificationDataURL': notificationFilePath}); + this.deviceDriver.cleanupRandomDirectory(notificationFilePath); } async setURLBlacklist(urlList) { diff --git a/detox/src/devices/Device.test.js b/detox/src/devices/Device.test.js index c5bbed481f..7f8bc04d30 100644 --- a/detox/src/devices/Device.test.js +++ b/detox/src/devices/Device.test.js @@ -218,7 +218,7 @@ describe('Device', () => { it(`relaunchApp() with userNofitication should send the userNotification as a param in launchParams`, async () => { device = validDevice(); fs.existsSync.mockReturnValue(true); - device.deviceDriver.createPushNotificationJson = jest.fn(() => 'url'); + device.deviceDriver.createUserNotificationFile = jest.fn(() => 'url'); await device.relaunchApp({userNotification: 'json'}); @@ -352,7 +352,7 @@ describe('Device', () => { device = validDevice(); await device.sendUserNotification('notif'); - expect(device.deviceDriver.createPushNotificationJson).toHaveBeenCalledTimes(1) + expect(device.deviceDriver.createUserNotificationFile).toHaveBeenCalledTimes(1) expect(device.deviceDriver.deliverPayload).toHaveBeenCalledTimes(1); }); @@ -517,7 +517,7 @@ describe('Device', () => { device = validDevice(); device.deviceDriver.getBundleIdFromBinary.mockReturnValue('test.bundle'); device.deviceDriver.launch.mockReturnValueOnce(processId).mockReturnValueOnce(processId); - device.deviceDriver.createPushNotificationJson = () => "url"; + device.deviceDriver.createUserNotificationFile = () => "url"; await device.prepare({launchApp: true}); await device.launchApp(launchParams); diff --git a/detox/src/devices/DeviceDriverBase.js b/detox/src/devices/DeviceDriverBase.js index 6c9948ea54..3eea9f1dad 100644 --- a/detox/src/devices/DeviceDriverBase.js +++ b/detox/src/devices/DeviceDriverBase.js @@ -1,5 +1,6 @@ const _ = require('lodash'); const fs = require('fs'); +const os = require('os'); const path = require('path'); class DeviceDriverBase { @@ -59,7 +60,7 @@ class DeviceDriverBase { return await this.client.reloadReactNative(); } - createPushNotificationJson(notification) { + createUserNotificationFile(notification) { } @@ -98,16 +99,46 @@ class DeviceDriverBase { defaultLaunchArgsPrefix() { return ''; } + + createRandomDirectory() { + const randomDir = fs.mkdtempSync(path.join(os.tmpdir(), 'detoxrand-')); + this.ensureDirectoryExistence(randomDir); + return randomDir; + } + + cleanupRandomDirectory(fileOrDir) { + if(path.basename(fileOrDir).startsWith('detoxrand-')) { + this._whyIsThereNoRMRFInNode(fileOrDir); + return + } + + this.cleanupRandomDirectory(path.dirname(fileOrDir)); + } + + _whyIsThereNoRMRFInNode(path) { + if(fs.existsSync(path)) { + fs.readdirSync(path).forEach(function(file, index){ + var curPath = path + "/" + file; + if (fs.lstatSync(curPath).isDirectory()) { + _whyIsThereNoRMRFInNode(curPath); + } else { + fs.unlinkSync(curPath); + } + }); + fs.rmdirSync(path); + } + } - ensureDirectoryExistence(filePath) { - const dirname = path.dirname(filePath); - if (fs.existsSync(dirname)) { - return true; + ensureDirectoryExistence(dirPath) { + if (fs.existsSync(dirPath)) { + return; } + + const dirOfDir = path.dirname(dirPath); - this.ensureDirectoryExistence(dirname); - fs.mkdirSync(dirname); - return true; + this.ensureDirectoryExistence(dirOfDir); + fs.mkdirSync(dirOfDir); + return; } getBundleIdFromBinary(appPath) { diff --git a/detox/src/devices/IosDriver.js b/detox/src/devices/IosDriver.js index d67a09652b..f305f59fb0 100644 --- a/detox/src/devices/IosDriver.js +++ b/detox/src/devices/IosDriver.js @@ -18,9 +18,8 @@ class IosDriver extends DeviceDriverBase { this.expect.exportGlobals(); } - createPushNotificationJson(notification) { - const notificationFilePath = path.join(__dirname, `detox`, `notifications`, `notification.json`); - this.ensureDirectoryExistence(notificationFilePath); + createUserNotificationFile(notification) { + const notificationFilePath = path.join(this.createRandomDirectory(), `notification.json`); fs.writeFileSync(notificationFilePath, JSON.stringify(notification, null, 2)); return notificationFilePath; } diff --git a/detox/test/e2e/k-user-notifications.js b/detox/test/e2e/k-user-notifications.js index 48a9703155..2748fc41b2 100644 --- a/detox/test/e2e/k-user-notifications.js +++ b/detox/test/e2e/k-user-notifications.js @@ -9,21 +9,21 @@ describe(':ios: User Notifications', () => { await expect(element(by.text('From calendar'))).toBeVisible(); }); - it.only('Background push notification', async () => { + it('Background push notification', async () => { await device.launchApp({newInstance: true}); await device.sendToHome(); await device.launchApp({newInstance: false, userNotification: userNotificationPushTrigger}); await expect(element(by.text('From push'))).toBeVisible(); }); - it.only('Background calendar notification', async () => { + it('Background calendar notification', async () => { await device.launchApp({newInstance: true}); await device.sendToHome(); await device.launchApp({newInstance: false, userNotification: userNotificationCalendarTrigger}); await expect(element(by.text('From calendar'))).toBeVisible(); }); - it('Foreground push notifications', async () => { + it.only('Foreground push notifications', async () => { await device.launchApp({newInstance: true}); await device.sendUserNotification(userNotificationCalendarTrigger); await expect(element(by.text('From calendar'))).toBeVisible(); diff --git a/detox/test/e2e/o-urls.js b/detox/test/e2e/o-urls.js index 8c0740bb3b..893e250edd 100644 --- a/detox/test/e2e/o-urls.js +++ b/detox/test/e2e/o-urls.js @@ -13,7 +13,7 @@ describe('Open URLs', () => { await expect(element(by.text(url))).toBeVisible(); }); - it.only('device.launchApp({url: url}) should trigger open url handling in app when app is in background', async () => { + it('device.launchApp({url: url}) should trigger open url handling in app when app is in background', async () => { const url = 'detoxtesturlscheme://such-string'; await device.launchApp({newInstance: true}); await device.sendToHome();