Repository: wix/Detox
Pull request #2595 (2021-01-15 12:23:37)

feat(config): --debug-synchronization=10000 by default
- [x] This change has been discussed in issue #1741 and the solution has been agreed upon with maintainers.

---

**Description:**

The resurrection of the pre-existing PR, originally by @rotemmiz.

* Updated the docs
* Added unit tests
* Use `--debug-synchronization false` instead of the original `off`, to be in line with `yargs` logic, which already processes `true` value (as 3000, due to compatibility reasons).



Patch:
diff --git a/detox/src/configuration/composeSessionConfig.js b/detox/src/configuration/composeSessionConfig.js
index a8b430b05..61238c82b 100644
--- a/detox/src/configuration/composeSessionConfig.js
+++ b/detox/src/configuration/composeSessionConfig.js
@@ -35,7 +35,7 @@ async function composeSessionConfig({ errorBuilder, cliConfig, detoxConfig, devi
     }
   }
 
-  if (cliConfig.debugSynchronization > 0) {
+  if (Number.parseInt(cliConfig.debugSynchronization, 10) >= 0) {
     session.debugSynchronization = +cliConfig.debugSynchronization;
   }
 
@@ -43,7 +43,7 @@ async function composeSessionConfig({ errorBuilder, cliConfig, detoxConfig, devi
     autoStart: !session.server,
     server: `ws://localhost:${await getPort()}`,
     sessionId: uuid.UUID(),
-    debugSynchronization: false,
+    debugSynchronization: 10000,
 
     ...session,
   };
diff --git a/detox/src/devices/DeviceRegistry.js b/detox/src/devices/DeviceRegistry.js
index d46def564..02f6783d7 100644
--- a/detox/src/devices/DeviceRegistry.js
+++ b/detox/src/devices/DeviceRegistry.js
@@ -70,7 +70,9 @@ class DeviceRegistry {
   async disposeDevice(getDeviceHandle) {
     await this._lockfile.exclusively(async () => {
       const deviceId = await safeAsync(getDeviceHandle);
-      this._toggleDeviceStatus(deviceId, false);
+      if (deviceId) {
+        this._toggleDeviceStatus(deviceId, false);
+      }
     });
   }
 
diff --git a/docs/APIRef.DetoxCLI.md b/docs/APIRef.DetoxCLI.md
index e54e2886e..ed93c270f 100644
--- a/docs/APIRef.DetoxCLI.md
+++ b/docs/APIRef.DetoxCLI.md
@@ -74,7 +74,7 @@ Initiating your test suite. <sup>[[1]](#notice-passthrough)</sup>
 | -o, --runner-config \<config\>                | Test runner config file, defaults to 'e2e/mocha.opts' for mocha and 'e2e/config.json' for jest. |
 | -n, --device-name [name]                      | Override the device name specified in a configuration. Useful for running a single build configuration on multiple devices. |
 | -l, --loglevel [value]                        | Log level: fatal, error, warn, info, verbose, trace |
-| -d, --debug-synchronization \<value\>         | When an action/expectation takes a significant amount time use this option to print device synchronization status. The status will be printed if the action takes more than [value]ms to complete |
+| -d, --debug-synchronization \<value\>         | [iOS Only] Customize how long an action/expectation can take to complete before Detox starts querying the app why it is busy. By default, the app status will be printed if the action takes more than 10s to complete. |
 | -a, --artifacts-location \<path\>             | Artifacts (logs, screenshots, etc) root directory.<sup>[[2]](#notice-artifacts)</sup> |
 | --record-logs [failing/all/none]              | Save logs during each test to artifacts directory. Pass "failing" to save logs of failing tests only. The default value is **none**. |
 | --take-screenshots [manual/failing/all/none]  | Save screenshots before and after each test to artifacts directory. Pass "failing" to save screenshots of failing tests only. The default value is **manual**. |


Test patch:
diff --git a/detox/local-cli/test.test.js b/detox/local-cli/test.test.js
index 5416d45b6..f20750104 100644
--- a/detox/local-cli/test.test.js
+++ b/detox/local-cli/test.test.js
@@ -117,16 +117,16 @@ describe('CLI', () => {
       expect(cliCall().command).toContain('--cleanup')
     });
 
-    test.each([['-d'], ['--debug-synchronization']])('%s <value> should be passed as CLI argument', async (__debug_synchronization) => {
-      await run(`${__debug_synchronization} 5000`);
-      expect(cliCall().command).toContain('--debug-synchronization 5000')
-    });
-
     test.each([['-d'], ['--debug-synchronization']])('%s <value> should have default value = 3000', async (__debug_synchronization) => {
       await run(`${__debug_synchronization}`);
       expect(cliCall().command).toContain('--debug-synchronization 3000')
     });
 
+    test.each([['-d'], ['--debug-synchronization']])('%s <value> should be passed as 0 when given false', async (__debug_synchronization) => {
+      await run(`${__debug_synchronization} false`);
+      expect(cliCall().command).toContain('--debug-synchronization 0')
+    });
+
     test.each([['-a'], ['--artifacts-location']])('%s <value> should be passed as CLI argument', async (__artifacts_location) => {
       await run(`${__artifacts_location} someLocation`);
       expect(cliCall().command).toContain('--artifacts-location someLocation')
@@ -402,6 +402,11 @@ describe('CLI', () => {
       expect(cliCall().env).toEqual(expect.objectContaining({ DETOX_DEBUG_SYNCHRONIZATION: 5000 }));
     });
 
+    test.each([['-d'], ['--debug-synchronization']])('%s <value> should be passed as 0 when given false', async (__debug_synchronization) => {
+      await run(`${__debug_synchronization} false`);
+      expect(cliCall().env).toEqual(expect.objectContaining({ DETOX_DEBUG_SYNCHRONIZATION: 0 }));
+    });
+
     test.each([['-d'], ['--debug-synchronization']])('%s <value> should have default value = 3000', async (__debug_synchronization) => {
       await run(`${__debug_synchronization}`);
       expect(cliCall().env).toEqual(expect.objectContaining({ DETOX_DEBUG_SYNCHRONIZATION: 3000 }));
diff --git a/detox/local-cli/utils/testCommandArgs.js b/detox/local-cli/utils/testCommandArgs.js
index d073af121..6d98e7abf 100644
--- a/detox/local-cli/utils/testCommandArgs.js
+++ b/detox/local-cli/utils/testCommandArgs.js
@@ -48,10 +48,14 @@ module.exports = {
     alias: 'debug-synchronization',
     group: 'Debugging:',
     coerce(value) {
-      if (value == null || value === false || value === 'false') {
+      if (value == null) {
         return undefined;
       }
 
+      if (value === false || value === 'false') {
+        return 0;
+      }
+
       if (value === true || value === 'true') {
         return 3000;
       }
@@ -59,8 +63,8 @@ module.exports = {
       return Number.isNaN(+value) ? value : +value;
     },
     describe:
-      'When an action/expectation takes a significant amount of time use this option to print device synchronization status.' +
-      'The status will be printed if the action takes more than [value]ms to complete',
+      '[iOS Only] Customize how long an action/expectation can take to complete before Detox starts querying the app why it is busy. ' +
+      'By default, the app status will be printed if the action takes more than 10s to complete.'
   },
   a: {
     alias: 'artifacts-location',
diff --git a/detox/src/configuration/composeSessionConfig.test.js b/detox/src/configuration/composeSessionConfig.test.js
index eff14c342..f28be25a5 100644
--- a/detox/src/configuration/composeSessionConfig.test.js
+++ b/detox/src/configuration/composeSessionConfig.test.js
@@ -24,7 +24,7 @@ describe('composeSessionConfig', () => {
   it('should generate a default config', async () => {
     expect(await compose()).toEqual({
       autoStart: true,
-      debugSynchronization: false,
+      debugSynchronization: 10000,
       server: expect.any(String),
       sessionId: expect.any(String),
     });
@@ -150,9 +150,9 @@ describe('composeSessionConfig', () => {
 
   describe('debugSynchronization', function () {
     describe('by default', () => {
-      it('should be false', async () => {
+      it('should be 10000ms', async () => {
         expect(await compose()).toMatchObject({
-          debugSynchronization: false,
+          debugSynchronization: 10000,
         });
       });
     });
@@ -167,12 +167,12 @@ describe('composeSessionConfig', () => {
 
     describe('when defined in global config', () => {
       beforeEach(() => {
-        detoxConfig.session = { debugSynchronization: 10000 };
+        detoxConfig.session = { debugSynchronization: 9999 };
       });
 
       it('should use that value', async () => {
         expect(await compose()).toMatchObject({
-          debugSynchronization: 10000,
+          debugSynchronization: 9999,
         });
       });
 
@@ -188,13 +188,24 @@ describe('composeSessionConfig', () => {
         });
 
         describe('and in CLI config', () => {
-          beforeEach(() => {
-            cliConfig.debugSynchronization = 3000;
+          it('should use that value if it is valid', async () => {
+            cliConfig.debugSynchronization = '0';
+            expect(await compose()).toMatchObject({
+              debugSynchronization: 0,
+            });
+          });
+
+          it('should ignore that value if it is invalid', async () => {
+            cliConfig.debugSynchronization = 'true';
+            expect(await compose()).toMatchObject({
+              debugSynchronization: 20000,
+            });
           });
 
-          it('should use that value', async () => {
+          it('should ignore that value if it is empty', async () => {
+            cliConfig.debugSynchronization = '';
             expect(await compose()).toMatchObject({
-              debugSynchronization: 3000,
+              debugSynchronization: 20000,
             });
           });
         });
diff --git a/detox/src/devices/DeviceRegistry.test.js b/detox/src/devices/DeviceRegistry.test.js
index 9f2e20487..39e33c402 100644
--- a/detox/src/devices/DeviceRegistry.test.js
+++ b/detox/src/devices/DeviceRegistry.test.js
@@ -224,6 +224,10 @@ describe('DeviceRegistry', () => {
       await expectNotIncludedInReadDevicesList(deviceHandleAsClassInstance);
     });
 
+    it('should not fail when there were no actual device to dispose', async () => {
+      await expect(disposeDevice(undefined)).resolves.not.toThrowError();
+    });
+
     describe('.reset() method', () => {
       it('should create a lock file with an empty array if it does not exist', async () => {
         expect(await fs.exists(lockfilePath)).toBe(false);