Repository: wix/Detox
Pull request #2693 (2021-03-17 15:52:35)

fix: use Node.js resolution for Detox config paths
- [x] This change has been discussed in issue #2550 and the solution has been agreed upon with maintainers.

---

**Description:**

This is a preparation towards extendable presets - now resolving Detox config paths via Node.js resolution mechanism.
Legacy configuration paths will be getting a warning:

```
Cannot resolve Detox config path by using Node.js require() mechanism:
require("node_modules/my-preset/index.js")

Detox now will resort to legacy filesystem-based path resolution.
Please fix your config path, so it conforms to `require(modulePath)` resolution.
```

Also, here we've got a fix.

Now Detox will be reporting correct errors when trying to evaluate something erroneous in a user-created JS (not JSON) Detox config. Previously, we incorrectly handled `Cannot find module` error. Why? So, if there was an inner `require` that failed, we were handling it like the user had provided an incorrect Detox config path, which is not always a true presumption.

Before:

![image](https://user-images.githubusercontent.com/1962469/111499465-a7a77a80-874b-11eb-9ffd-c58197a2f1c2.png)

After:

![image](https://user-images.githubusercontent.com/1962469/111499498-ad9d5b80-874b-11eb-8e56-06c81ff51683.png)



Patch:
diff --git a/detox/src/configuration/__mocks__/configuration/badconfig/detox.config.js b/detox/src/configuration/__mocks__/configuration/badconfig/detox.config.js
new file mode 100644
index 000000000..37c1db02f
--- /dev/null
+++ b/detox/src/configuration/__mocks__/configuration/badconfig/detox.config.js
@@ -0,0 +1,2 @@
+/* eslint-disable */
+module.exports = require('something-that-does-not-exist');
diff --git a/detox/src/configuration/index.js b/detox/src/configuration/index.js
index fef317617..8505d3ab1 100644
--- a/detox/src/configuration/index.js
+++ b/detox/src/configuration/index.js
@@ -11,7 +11,7 @@ const composeSessionConfig = require('./composeSessionConfig');
 const selectConfiguration = require('./selectConfiguration');
 
 async function composeDetoxConfig({
-  cwd,
+  cwd = process.cwd(),
   argv,
   override,
   userParams,
diff --git a/detox/src/configuration/loadExternalConfig.js b/detox/src/configuration/loadExternalConfig.js
index 4c2dc974c..40a5a27b8 100644
--- a/detox/src/configuration/loadExternalConfig.js
+++ b/detox/src/configuration/loadExternalConfig.js
@@ -1,6 +1,8 @@
 const fs = require('fs-extra');
 const path = require('path');
 const findUp = require('find-up');
+const resolveFrom = require('resolve-from');
+const log = require('../utils/logger').child({ __filename });
 
 async function locateExternalConfig(cwd) {
   return findUp([
@@ -28,6 +30,28 @@ async function loadConfig(configPath) {
   };
 }
 
+async function resolveConfigPath(configPath, cwd) {
+  if (!configPath) {
+    return locateExternalConfig(cwd);
+  }
+
+  const viaNodeResolution = resolveFrom.silent(cwd, configPath);
+  if (viaNodeResolution) {
+    return viaNodeResolution;
+  }
+
+  if (fs.existsSync(configPath)) {
+    log.warn('Cannot resolve Detox config path by using Node.js require() mechanism:\n' +
+      `require(${JSON.stringify(configPath)})\n\n` +
+      'Detox now will resort to legacy filesystem-based path resolution.\n' +
+      'Please fix your config path, so it conforms to `require(modulePath)` resolution.');
+
+    return path.resolve(configPath);
+  }
+
+  return null;
+}
+
 /**
  * @param {DetoxConfigErrorComposer} errorComposer
  * @param {string} configPath
@@ -35,9 +59,7 @@ async function loadConfig(configPath) {
  * @returns {Promise<null|{filepath: *, config: any}>}
  */
 async function loadExternalConfig({ errorComposer, configPath, cwd }) {
-  const resolvedConfigPath = configPath
-    ? path.resolve(configPath)
-    : await locateExternalConfig(cwd);
+  const resolvedConfigPath = await resolveConfigPath(configPath, cwd);
 
   if (resolvedConfigPath) {
     errorComposer.setDetoxConfigPath(resolvedConfigPath);
@@ -45,12 +67,12 @@ async function loadExternalConfig({ errorComposer, configPath, cwd }) {
     try {
       return await loadConfig(resolvedConfigPath);
     } catch (e) {
-      if (/Cannot find module|ENOENT/.test(`${e}`)) {
-        throw errorComposer.noConfigurationAtGivenPath();
-      } else {
-        throw errorComposer.failedToReadConfiguration(e);
-      }
+      throw errorComposer.failedToReadConfiguration(e);
     }
+  } else if (configPath) {
+    throw errorComposer
+      .setDetoxConfigPath(configPath)
+      .noConfigurationAtGivenPath();
   }
 
   return null;


Test patch:
diff --git a/detox/src/configuration/loadExternalConfig.test.js b/detox/src/configuration/loadExternalConfig.test.js
index 380ef6204..94f0ac1c4 100644
--- a/detox/src/configuration/loadExternalConfig.test.js
+++ b/detox/src/configuration/loadExternalConfig.test.js
@@ -1,21 +1,28 @@
 const path = require('path');
 const os = require('os');
-const DetoxConfigErrorComposer = require('../errors/DetoxConfigErrorComposer');
 
 describe('loadExternalConfig', () => {
   const DIR_PACKAGEJSON = path.join(__dirname, '__mocks__/configuration/packagejson');
   const DIR_PRIORITY = path.join(__dirname, '__mocks__/configuration/priority');
+  const DIR_BADCONFIG = path.join(__dirname, '__mocks__/configuration/badconfig');
 
+  let DetoxConfigErrorComposer;
   /** @type {DetoxConfigErrorComposer} */
   let errorComposer;
   let loadExternalConfig;
+  let logger;
 
   beforeEach(() => {
+    jest.mock('../utils/logger');
+    logger = require('../utils/logger');
+
+    DetoxConfigErrorComposer = require('../errors/DetoxConfigErrorComposer');
     errorComposer = new DetoxConfigErrorComposer();
 
     loadExternalConfig = (opts) => require('./loadExternalConfig')({
-      ...opts,
+      cwd: process.cwd(),
       errorComposer,
+      ...opts,
     });
   });
 
@@ -24,6 +31,7 @@ describe('loadExternalConfig', () => {
 
     expect(filepath).toBe(path.join(DIR_PRIORITY, '.detoxrc.js'))
     expect(config).toMatchObject({ configurations: expect.anything() });
+    expect(logger.warn).not.toHaveBeenCalled();
   });
 
   it('should implicitly use package.json, even if there is no .detoxrc', async () => {
@@ -31,6 +39,7 @@ describe('loadExternalConfig', () => {
 
     expect(filepath).toBe(path.join(DIR_PACKAGEJSON, 'package.json'))
     expect(config).toMatchObject({ configurations: expect.anything() });
+    expect(logger.warn).not.toHaveBeenCalled();
   });
 
   it('should return an empty result if a config cannot be implicitly found', async () => {
@@ -44,16 +53,33 @@ describe('loadExternalConfig', () => {
 
     expect(filepath).toBe(configPath)
     expect(config).toMatchObject({ configurations: expect.anything() });
+    expect(logger.warn).not.toHaveBeenCalled();
   });
 
-  it('should throw if the explicitly given config is not found', async () => {
+  it('should throw noConfigurationAtGivenPath error if the explicitly given config is not found', async () => {
     const configPath = path.join(DIR_PRIORITY, 'non-existent.json');
 
     await expect(loadExternalConfig({ configPath })).rejects.toThrowError(
-      errorComposer.noConfigurationAtGivenPath()
+      errorComposer.setDetoxConfigPath(configPath).noConfigurationAtGivenPath()
     );
   });
 
+  it('should throw failedToReadConfiguration error if the implicitly resolved config throws "Cannot find module..."', async () => {
+    await expect(loadExternalConfig({ cwd: DIR_BADCONFIG })).rejects.toThrowError('something-that-does-not-exist');
+  });
+
+  it('should fall back to fs-based config path resolution', () => {
+    const absoluteConfigPath = path.join(DIR_PRIORITY, 'detox-config.json');
+    const relativeConfigPath = path.relative(process.cwd(), absoluteConfigPath);
+
+    const absoluteConfig = loadExternalConfig({ configPath: absoluteConfigPath });
+    expect(logger.warn).not.toHaveBeenCalled();
+    const relativeConfig = loadExternalConfig({ configPath: relativeConfigPath });
+    expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('legacy filesystem'));
+
+    expect(absoluteConfig).toEqual(relativeConfig);
+  });
+
   it('should rethrow if an unexpected error occurs', async () => {
     const configPath = os.homedir();