Repository: wix/Detox
Pull request #2403 (2020-10-12 13:46:26)

test(win32): unit tests should pass on Windows
- [x] This is a small change 

---

**Description:**

Struggling here for the justice, to make sure our fellow contributors on Windows can run the unit tests with zero manual patches.

Also, the PR removes redundant `\` escaping on Windows (when using CMD.exe). I've overlooked this possibility in the initial PR for improving the escaping algorithm — unfortunately, had to split the table unit tests for escaping implementations for Shell and CMD.exe.


Patch:
diff --git a/detox/src/utils/shellUtils.js b/detox/src/utils/shellUtils.js
index 13a4085d2..2601d6f63 100644
--- a/detox/src/utils/shellUtils.js
+++ b/detox/src/utils/shellUtils.js
@@ -26,28 +26,38 @@ function isRunningInCMDEXE() {
          /* istanbul ignore next */ !process.env['SHELL'];
 }
 
-const UNSAFE = /[\s!"#$&'()*;<=>^?`{,}|~\[\\\]]/m;
+const UNSAFE_SHELL = /[\s!"#$&'()*;<=>^?`{,}|~\[\\\]]/m;
+const UNSAFE_CMD = /[\s!"#$&'()*;<=>^?`{,}|~\[\]]/m;
+
 /* @see https://unix.stackexchange.com/a/357932 */
 function hasUnsafeShellChars(str) {
-  return UNSAFE.test(str);
+  return UNSAFE_SHELL.test(str);
 }
 
-function autoEscapeCmd(str) {
+function autoEscapeShell(str) {
   if (!hasUnsafeShellChars(str)) {
     return str;
   }
 
-  return escapeWithDoubleQuotedString(str);
+  return escapeWithSingleQuotedString(str);
 }
 
-function autoEscapeShell(str) {
-  if (!hasUnsafeShellChars(str)) {
+function hasUnsafeCMDChars(str) {
+  return UNSAFE_CMD.test(str);
+}
+
+function autoEscapeCmd(str) {
+  if (!hasUnsafeCMDChars(str)) {
     return str;
   }
 
-  return escapeWithSingleQuotedString(str);
+  return escapeWithDoubleQuotedString(str);
 }
 
+const hasUnsafeChars = isRunningInCMDEXE()
+  /* istanbul ignore next */ ? hasUnsafeCMDChars
+  /* istanbul ignore next */ : hasUnsafeShellChars;
+
 const autoEscape = isRunningInCMDEXE()
   /* istanbul ignore next */ ? autoEscapeCmd
   /* istanbul ignore next */ : autoEscapeShell;
@@ -58,7 +68,10 @@ module.exports = {
   escapeWithSingleQuotedString,
   escapeWithDoubleQuotedString,
   isRunningInCMDEXE,
-  hasUnsafeShellChars,
+  hasUnsafeChars: Object.assign(hasUnsafeChars, {
+    cmd: hasUnsafeCMDChars,
+    shell: hasUnsafeShellChars,
+  }),
   autoEscape: Object.assign(autoEscape, {
     cmd: autoEscapeCmd,
     shell: autoEscapeShell,


Test patch:
diff --git a/detox/local-cli/test.test.js b/detox/local-cli/test.test.js
index aa197e291..90157fb3d 100644
--- a/detox/local-cli/test.test.js
+++ b/detox/local-cli/test.test.js
@@ -51,7 +51,14 @@ describe('CLI', () => {
       test('should pass --use-custom-logger true', () => expect(cliCall().command).toMatch(/--use-custom-logger true/));
       test('should not override process.env', () => expect(cliCall().env).toStrictEqual({}));
       test('should produce a default command (integration test)', () => {
-        const args = `--opts e2e/mocha.opts --grep :android: --invert --config-path ${detoxConfigPath} --use-custom-logger true`;
+        const quoteChar = !isInCMD() && detoxConfigPath.indexOf('\\') >= 0 ? `'` : '';
+        const args = [
+          `--opts`, `e2e/mocha.opts`,
+          `--grep`, `:android:`, `--invert`,
+          `--config-path`, quote(detoxConfigPath, quoteChar),
+          `--use-custom-logger`, `true`
+        ].join(' ');
+
         expect(cliCall().command).toBe(`mocha ${args} e2e`);
       });
     });
@@ -217,9 +224,9 @@ describe('CLI', () => {
     });
 
     test.each([
-      [`--runner-config "mocha configs/.mocharc"`, `--config 'mocha configs/.mocharc'`],
-      [`--artifacts-location "artifacts dir/"`, `--artifacts-location 'artifacts dir/'`],
-      [`--device-name "iPhone X"`, `--device-name 'iPhone X'`],
+      [`--runner-config "mocha configs/.mocharc"`, `--config ${quote('mocha configs/.mocharc')}`],
+      [`--artifacts-location "artifacts dir/"`, `--artifacts-location ${quote('artifacts dir/')}`],
+      [`--device-name "iPhone X"`, `--device-name ${quote('iPhone X')}`],
       [`"e2e tests/first test.spec.js"`, `"e2e tests/first test.spec.js"`],
     ])('should escape %s when forwarding it as a CLI argument', async (cmd, expected) => {
       await run(cmd);
@@ -239,7 +246,7 @@ describe('CLI', () => {
       });
 
       test('should produce a default command (integration test, ios)', () => {
-        const args = `--config e2e/config.json --testNamePattern '^((?!:android:).)*$' --maxWorkers 1`;
+        const args = `--config e2e/config.json --testNamePattern ${quote('^((?!:android:).)*$')} --maxWorkers 1`;
         expect(cliCall().command).toBe(`jest ${args} e2e`);
       });
 
@@ -260,7 +267,7 @@ describe('CLI', () => {
       });
 
       test('should produce a default command (integration test)', () => {
-        const args = `--config e2e/config.json --testNamePattern '^((?!:ios:).)*$' --maxWorkers 1`;
+        const args = `--config e2e/config.json --testNamePattern ${quote('^((?!:ios:).)*$')} --maxWorkers 1`;
         expect(cliCall().command).toBe(`jest ${args} e2e`);
       });
 
@@ -285,11 +292,11 @@ describe('CLI', () => {
         detoxConfig.configurations.androidTest.type = 'android.emulator';
 
         await run(`${__configuration} androidTest`);
-        expect(cliCall(0).command).toContain(`--testNamePattern '^((?!:ios:).)*$'`);
+        expect(cliCall(0).command).toContain(`--testNamePattern ${quote('^((?!:ios:).)*$')}`);
         expect(cliCall(0).env.configuration).toBe('androidTest');
 
         await run(`${__configuration} iosTest`);
-        expect(cliCall(1).command).toContain(`--testNamePattern '^((?!:android:).)*$'`);
+        expect(cliCall(1).command).toContain(`--testNamePattern ${quote('^((?!:android:).)*$')}`);
         expect(cliCall(1).env.configuration).toBe('iosTest');
       }
     );
@@ -578,7 +585,7 @@ describe('CLI', () => {
     });
 
     test.each([
-      [`--testNamePattern "should tap"`, `--testNamePattern 'should tap'`],
+      [`--testNamePattern "should tap"`, `--testNamePattern ${quote('should tap')}`],
       [`"e2e tests/first test.spec.js"`, `"e2e tests/first test.spec.js"`],
     ])('should escape %s when forwarding it as a CLI argument', async (cmd, expected) => {
       await run(cmd);
@@ -705,4 +712,12 @@ describe('CLI', () => {
   function singleConfig() {
     return Object.values(detoxConfig.configurations)[0];
   }
+
+  function isInCMD() {
+    return process.platform === 'win32' && !process.env.SHELL;
+  }
+
+  function quote(s, q = isInCMD() ? `"` : `'`) {
+    return q + s + q;
+  }
 });
diff --git a/detox/src/utils/environment.test.js b/detox/src/utils/environment.test.js
index 1138d5177..e90bb599b 100644
--- a/detox/src/utils/environment.test.js
+++ b/detox/src/utils/environment.test.js
@@ -92,65 +92,65 @@ describe('Environment', () => {
         delete process.env.ANDROID_HOME;
 
         const sdkPath = Environment.getAndroidSDKPath();
-        expect(sdkPath).toEqual('');
+        expect(sdkPath).toBe('');
       });
 
       it(`should return $ANDROID_HOME if it is set`, () => {
         delete process.env.ANDROID_SDK_ROOT;
-        process.env.ANDROID_HOME = 'path/to/android/home';
+        process.env.ANDROID_HOME = path.normalize('path/to/android/home');
 
         const sdkPath = Environment.getAndroidSDKPath();
-        expect(sdkPath).toEqual('path/to/android/home');
+        expect(sdkPath).toBe(process.env.ANDROID_HOME);
       });
 
       it(`should return $ANDROID_SDK_ROOT if it is set`, () => {
         delete process.env.ANDROID_HOME;
-        process.env.ANDROID_SDK_ROOT = 'path/to/sdk/root';
+        process.env.ANDROID_SDK_ROOT = path.normalize('path/to/sdk/root');
 
         const sdkPath = Environment.getAndroidSDKPath();
-        expect(sdkPath).toEqual('path/to/sdk/root');
+        expect(sdkPath).toBe(process.env.ANDROID_SDK_ROOT);
       });
 
       it(`should prefer $ANDROID_SDK_ROOT, if both $ANDROID_SDK_ROOT and $ANDROID_HOME are set`, () => {
-        process.env.ANDROID_SDK_ROOT = 'path/to/sdk/root';
-        process.env.ANDROID_HOME = 'path/to/android/home';
+        process.env.ANDROID_SDK_ROOT = path.normalize('path/to/sdk/root');
+        process.env.ANDROID_HOME = path.normalize('path/to/android/home');
 
         const sdkPath = Environment.getAndroidSDKPath();
-        expect(sdkPath).toEqual('path/to/sdk/root');
+        expect(sdkPath).toBe(process.env.ANDROID_SDK_ROOT);
       });
     });
 
     describe('getAvdManagerPath', () => {
       it('should return path to AVD-manager executable', () => {
-        process.env.ANDROID_SDK_ROOT = 'mock/path/to/sdk';
+        process.env.ANDROID_SDK_ROOT = path.normalize('mock/path/to/sdk');
 
         const avdManagerPath = Environment.getAvdManagerPath();
-        expect(avdManagerPath).toEqual('mock/path/to/sdk/tools/bin/avdmanager');
+        expect(avdManagerPath).toBe(path.join(process.env.ANDROID_SDK_ROOT, 'tools/bin/avdmanager'));
       });
 
       it('should fall back to using ANDROID_HOME instead of ANDROID_SDK_ROOT', () => {
         delete process.env.ANDROID_SDK_ROOT;
-        process.env.ANDROID_HOME = 'mock/path/to/sdk';
+        process.env.ANDROID_HOME = path.normalize('mock/path/to/sdk');
 
         const avdManagerPath = Environment.getAvdManagerPath();
-        expect(avdManagerPath).toEqual('mock/path/to/sdk/tools/bin/avdmanager');
+        expect(avdManagerPath).toBe(path.join(process.env.ANDROID_HOME, 'tools/bin/avdmanager'));
       });
     });
 
     describe('getAndroidSdkManagerPath', () => {
       it('should return path to SDK-manager executable', () => {
-        process.env.ANDROID_SDK_ROOT = 'mock/path/to/sdk';
+        process.env.ANDROID_SDK_ROOT = path.normalize('mock/path/to/sdk');
 
         const sdkManagerPath = Environment.getAndroidSdkManagerPath();
-        expect(sdkManagerPath).toEqual('mock/path/to/sdk/tools/bin/sdkmanager');
+        expect(sdkManagerPath).toBe(path.join(process.env.ANDROID_SDK_ROOT, 'tools/bin/sdkmanager'));
       });
 
       it('should fall back to using ANDROID_HOME instead of ANDROID_SDK_ROOT', () => {
         delete process.env.ANDROID_SDK_ROOT;
-        process.env.ANDROID_HOME = 'mock/path/to/sdk';
+        process.env.ANDROID_HOME = path.normalize('mock/path/to/sdk');
 
         const sdkManagerPath = Environment.getAndroidSdkManagerPath();
-        expect(sdkManagerPath).toEqual('mock/path/to/sdk/tools/bin/sdkmanager');
+        expect(sdkManagerPath).toBe(path.join(process.env.ANDROID_HOME, 'tools/bin/sdkmanager'));
       });
     });
 
diff --git a/detox/src/utils/errorUtils.test.js b/detox/src/utils/errorUtils.test.js
index c7cf8b3d8..bb85c9220 100644
--- a/detox/src/utils/errorUtils.test.js
+++ b/detox/src/utils/errorUtils.test.js
@@ -54,8 +54,10 @@ describe('replaceErrorStack(source, target)', () => {
 
 describe('createErrorWithUserStack()', () => {
   it('should not have /detox/src/ lines in stack', () => {
-    expect(new Error().stack).toContain('/detox/src/'); // sanity assertion
-    expect(errorUtils.createErrorWithUserStack()).not.toContain('/detox/src/');
+    expect(new Error().stack).toMatch(/[\\\/]detox[\\\/]src[\\\/]/m); // sanity assertion
+
+    expect(errorUtils.createErrorWithUserStack()).not.toContain('/detox/src/'); // POSIX
+    expect(errorUtils.createErrorWithUserStack()).not.toContain('\\detox\\src\\'); // WIN32
   });
 });
 
diff --git a/detox/src/utils/shellUtils.test.js b/detox/src/utils/shellUtils.test.js
index 745c8e0d2..6dd8a64cc 100644
--- a/detox/src/utils/shellUtils.test.js
+++ b/detox/src/utils/shellUtils.test.js
@@ -4,7 +4,7 @@ const {
   escapeWithSingleQuotedString,
   escapeWithDoubleQuotedString,
   isRunningInCMDEXE,
-  hasUnsafeShellChars,
+  hasUnsafeChars,
   autoEscape,
 } = require('./shellUtils');
 
@@ -51,46 +51,60 @@ describe('shellUtils', function() {
     });
   });
 
-  describe('hasUnsafeShellChars', () => {
-    test.each([
+  describe('hasUnsafeChars', () => {
+    const CASES = [
+    /* cmd    shell  input comment */
       /* pin-pointer tests */
-      [false, '',  'just an empty string'],
-      [true, ' ',  'a whitespace character'],
-      [true, '\t', 'a whitespace character'],
-      [true, '\n', 'a newline character'],
-      [true, '!',  'a history expansion'],
-      [true, '"',  'shell syntax'],
-      [true, '#',  'a comment start'],
-      [true, '$',  'shell syntax'],
-      [true, '&',  'shell syntax'],
-      [true, `'`,  'shell syntax'],
-      [true, '(',  'globs and wildcards'],
-      [true, ')',  'globs and wildcards'],
-      [true, '*',  'a sh wildcard'],
-      [true, ';',  'shell syntax'],
-      [true, '<',  'shell syntax'],
-      [true, '=',  'zsh syntax'],
-      [true, '>',  'shell syntax'],
-      [true, '?',  'a sh wildcard'],
-      [true, '[',  'a sh wildcard'],
-      [true, "\\", 'shell syntax'],
-      [true, ']',  'a sh wildcard'],
-      [true, '^',  'a history expansion, zsh wildcard'],
-      [true, '`',  'shell syntax'],
-      [true, '{',  'a brace expansion start'],
-      [true, ',',  'unsafe inside a brace expansion'],
-      [true, '}',  'a brace expansion end'],
-      [true, '|',  'shell syntax'],
-      [true, '~',  'a home directory expansion'],
-      [false, '-',  'almost safe, except when a filename begins with dash, it needs extra handling'],
-      [false, '.',  'almost safe, except that dot files are excluded from * globs by default.'],
-      [false, ':',  'almost safe, expect when it can indicate a remote file (hostname:filename)'],
-
+      [false, false, '',   'just an empty string'],
+      [true,  true,  ' ',  'a whitespace character'],
+      [true,  true,  '\t', 'a whitespace character'],
+      [true,  true,  '\n', 'a newline character'],
+      [true,  true,  '!',  'a history expansion'],
+      [true,  true,  '"',  'shell syntax'],
+      [true,  true,  '#',  'a comment start'],
+      [true,  true,  '$',  'shell syntax'],
+      [true,  true,  '&',  'shell syntax'],
+      [true,  true,  `'`,  'shell syntax'],
+      [true,  true,  '(',  'globs and wildcards'],
+      [true,  true,  ')',  'globs and wildcards'],
+      [true,  true,  '*',  'a sh wildcard'],
+      [true,  true,  ';',  'shell syntax'],
+      [true,  true,  '<',  'shell syntax'],
+      [true,  true,  '=',  'zsh syntax'],
+      [true,  true,  '>',  'shell syntax'],
+      [true,  true,  '?',  'a sh wildcard'],
+      [true,  true,  '[',  'a sh wildcard'],
+      [false, true,  "\\", 'shell syntax'],
+      [true,  true,  ']',  'a sh wildcard'],
+      [true,  true,  '^',  'a history expansion, zsh wildcard'],
+      [true,  true,  '`',  'shell syntax'],
+      [true,  true,  '{',  'a brace expansion start'],
+      [true,  true,  ',',  'unsafe inside a brace expansion'],
+      [true,  true,  '}',  'a brace expansion end'],
+      [true,  true,  '|',  'shell syntax'],
+      [true,  true,  '~',  'a home directory expansion'],
+      [false, false, '-', 'almost safe, except when a filename begins with dash, it needs extra handling'],
+      [false, false, '.', 'almost safe, except that dot files are excluded from * globs by default.'],
+      [false, false, ':', 'almost safe, expect when it can indicate a remote file (hostname:filename)'],
       /* integration tests */
-      [false, '123-abc-абв.test.js',  'just a mere test filename'],
-      [true, 'some tests/my test.js',  'a filename with spaces'],
-    ])('should return %j for %j because it is %s', (expected, str) => {
-      expect(hasUnsafeShellChars(str)).toBe(expected);
+      [false, false, '123-abc-абв.test.js',  'just a mere test filename'],
+      [true, true, 'some tests/my test.js',  'a filename with spaces'],
+    ];
+
+    describe('.cmd', () => {
+      const CMD_CASES = CASES.map(([expected, _shell, input, comment]) => [expected, input, comment]);
+
+      test.each(CMD_CASES)('should return %j for %j because it is %s', (expected, str, comment) => {
+        expect(hasUnsafeChars.cmd(str)).toBe(expected);
+      });
+    });
+
+    describe('.shell', () => {
+      const SHELL_CASES = CASES.map(([_cmd, expected, input, comment]) => [expected, input, comment]);
+
+      test.each(SHELL_CASES)('should return %j for %j because it is %s', (expected, str, comment) => {
+        expect(hasUnsafeChars.shell(str)).toBe(expected);
+      });
     });
   });