Merge pull request #1311 from apiaryio/cli-redesign

Re-design Dredd configuration handling

docs/usage-js.rst

@@ -26,48 +26,45 @@ As you can see, ``dredd.run`` is a function receiving another function as a call
 Configuration Object for Dredd Class
 ------------------------------------
 
-Let’s have a look at an example configuration first. (Please also see the :ref:`CLI options <usage-cli>` to read detailed information about the ``options`` attributes).
+Let’s have a look at an example configuration first. (Please also see the :ref:`CLI options <usage-cli>` to read detailed information about the list of available options).
 
 .. code-block:: javascript
 
    {
-     server: 'http://127.0.0.1:3000/api', // your URL to API endpoint the tests will run against
-     options: {
-       path: [],         // Required Array if Strings; filepaths to API description documents, can use glob wildcards
-       'dry-run': false, // Boolean, do not run any real HTTP transaction
-       names: false,     // Boolean, Print Transaction names and finish, similar to dry-run
-       loglevel: 'warning', // String, logging level (debug, warning, error, silent)
-       only: [],         // Array of Strings, run only transaction that match these names
-       header: [],       // Array of Strings, these strings are then added as headers (key:value) to every transaction
-       user: null,       // String, Basic Auth credentials in the form username:password
-       hookfiles: [],    // Array of Strings, filepaths to files containing hooks (can use glob wildcards)
-       reporter: ['dot', 'html'], // Array of possible reporters, see folder lib/reporters
-       output: [],       // Array of Strings, filepaths to files used for output of file-based reporters
-       'inline-errors': false, // Boolean, If failures/errors are display immediately in Dredd run
-       require: null,    // String, When using nodejs hooks, require the given module before executing hooks
-       color: true,
-     },
+     endpoint: 'http://127.0.0.1:3000/api', // your URL to API endpoint the tests will run against
+     path: [],         // Required Array if Strings; filepaths to API description documents, can use glob wildcards
+     'dry-run': false, // Boolean, do not run any real HTTP transaction
+     names: false,     // Boolean, Print Transaction names and finish, similar to dry-run
+     loglevel: 'warning', // String, logging level (debug, warning, error, silent)
+     only: [],         // Array of Strings, run only transaction that match these names
+     header: [],       // Array of Strings, these strings are then added as headers (key:value) to every transaction
+     user: null,       // String, Basic Auth credentials in the form username:password
+     hookfiles: [],    // Array of Strings, filepaths to files containing hooks (can use glob wildcards)
+     reporter: ['dot', 'html'], // Array of possible reporters, see folder lib/reporters
+     output: [],       // Array of Strings, filepaths to files used for output of file-based reporters
+     'inline-errors': false, // Boolean, If failures/errors are display immediately in Dredd run
+     require: null,    // String, When using nodejs hooks, require the given module before executing hooks
+     color: true,
      emitter: new EventEmitter(), // listen to test progress, your own instance of EventEmitter
      apiDescriptions: ['FORMAT: 1A\n# Sample API\n']
    }
 
+.. warning::
+   The usage of nested ``options`` key is deprecated. Please list options under the root of the configuration. 
+
+.. warning::
+   The top-level `server` property must be replaced by ``endpoint``. Do not confuse with the options `--server` option, that provides a server running command (i.e. ``npm start``).
+
 .. js:data:: configuration
 
-.. js:attribute:: configuration.server
+.. js:attribute:: configuration.endpoint
 
    The HTTP(S) address of the API server to test against the API description(s). A valid URL is expected, e.g. ``http://127.0.0.1:8000``
 
    :type: string
    :required: yes
 
-.. js:attribute:: configuration.options
-
-   Because :js:attr:`configuration.options.path` array is required, you must specify options. You’ll end with errors otherwise.
-
-   :type: object
-   :required: yes
-
-.. js:attribute:: configuration.options.path
+.. js:attribute:: configuration.path
 
    Array of paths or URLs to API description documents.
 

lib/CLI.js

@@ -1,3 +1,4 @@
+const R = require('ramda');
 const console = require('console'); // Stubbed in tests by proxyquire
 const fs = require('fs');
 const optimist = require('optimist');
@@ -322,18 +323,17 @@ ${packageData.name} v${packageData.version} \
   initConfig() {
     this.lastArgvIsApiEndpoint().takeRestOfParamsAsPath();
 
-    const configuration = {
+    const cliConfig = R.mergeDeepRight(this.argv, {
       server: this.server,
-      options: this.argv,
-    };
+    });
 
     // Push first argument (without some known configuration --key) into paths
-    if (!configuration.options.path) { configuration.options.path = []; }
-    configuration.options.path.push(this.argv._[0]);
+    if (!cliConfig.path) { cliConfig.path = []; }
+    cliConfig.path.push(this.argv._[0]);
 
-    configuration.custom = this.custom;
+    cliConfig.custom = this.custom;
 
-    return configuration;
+    return cliConfig;
   }
 
   initDredd(configuration) {

lib/Dredd.js

@@ -42,8 +42,8 @@ class Dredd {
     configureReporters(this.configuration, this.stats, this.transactionRunner);
 
     this.logger.debug('Resolving --require');
-    if (this.configuration.options.require) {
-      const requirePath = resolveModule(this.configuration.custom.cwd, this.configuration.options.require);
+    if (this.configuration.require) {
+      const requirePath = resolveModule(this.configuration.custom.cwd, this.configuration.require);
       try {
         // eslint-disable-next-line global-require, import/no-dynamic-require
         require(requirePath);
@@ -56,7 +56,7 @@ class Dredd {
 
     this.logger.debug('Resolving API descriptions locations');
     try {
-      this.files = resolveLocations(this.configuration.custom.cwd, this.configuration.options.path);
+      this.files = resolveLocations(this.configuration.custom.cwd, this.configuration.path);
     } catch (err) {
       callback(err, this.stats);
       return;

lib/HooksWorkerClient.js

@@ -12,7 +12,8 @@ const { spawn } = require('./childProcess');
 class HooksWorkerClient {
   constructor(runner) {
     this.runner = runner;
-    const { options } = this.runner.hooks.configuration;
+    const options = this.runner.hooks.configuration;
+
     this.language = options.language;
     this.timeout = options['hooks-worker-timeout'] || 5000;
     this.connectTimeout = options['hooks-worker-connect-timeout'] || 1500;
@@ -180,7 +181,7 @@ $ go get github.com/snikch/goodman/cmd/goodman
   }
 
   spawnHandler(callback) {
-    const pathGlobs = this.runner.hooks.configuration.options.hookfiles;
+    const pathGlobs = this.runner.hooks.configuration.hookfiles;
     const handlerCommandArgs = this.handlerCommandArgs.concat(pathGlobs);
 
     logger.debug(`Spawning '${this.language}' hooks handler process.`);

lib/TransactionRunner.js

@@ -47,7 +47,7 @@ class TransactionRunner {
       if (emitStartErr) { return callback(emitStartErr); }
 
       logger.debug('Sorting HTTP transactions');
-      transactions = this.configuration.options.sorted ? sortTransactions(transactions) : transactions;
+      transactions = this.configuration.sorted ? sortTransactions(transactions) : transactions;
 
       logger.debug('Configuring HTTP transactions');
       transactions = transactions.map(this.configureTransaction.bind(this));
@@ -224,7 +224,7 @@ class TransactionRunner {
     const { origin, request, response } = transaction;
 
     // Parse the server URL (just once, caching it in @parsedUrl)
-    if (!this.parsedUrl) { this.parsedUrl = this.parseServerUrl(configuration.server); }
+    if (!this.parsedUrl) { this.parsedUrl = this.parseServerUrl(configuration.endpoint); }
     const fullPath = this.getFullPath(this.parsedUrl.path, request.uri);
 
     const headers = headersArrayToObject(request.headers);
@@ -239,8 +239,8 @@ class TransactionRunner {
     }
 
     // Parse and add headers from the config to the transaction
-    if (configuration.options.header.length > 0) {
-      for (const header of configuration.options.header) {
+    if (configuration.header.length > 0) {
+      for (const header of configuration.header) {
         const splitIndex = header.indexOf(':');
         const headerKey = header.substring(0, splitIndex);
         const headerValue = header.substring(splitIndex + 1);
@@ -453,28 +453,28 @@ class TransactionRunner {
       transaction.test = test;
       this.failTransaction(transaction, `Failed in before hook: ${transaction.fail}`);
       return callback();
-    } if (this.configuration.options['dry-run']) {
+    } if (this.configuration['dry-run']) {
       reporterOutputLogger.info('Dry run. Not performing HTTP request');
       transaction.test = test;
       this.skipTransaction(transaction);
       return callback();
-    } if (this.configuration.options.names) {
+    } if (this.configuration.names) {
       reporterOutputLogger.info(transaction.name);
       transaction.test = test;
       this.skipTransaction(transaction);
       return callback();
-    } if ((this.configuration.options.method.length > 0) && !(Array.from(this.configuration.options.method).includes(transaction.request.method))) {
+    } if ((this.configuration.method.length > 0) && !(Array.from(this.configuration.method).includes(transaction.request.method))) {
       logger.debug(`\
-Only ${(Array.from(this.configuration.options.method).map(m => m.toUpperCase())).join(', ')}\
+Only ${(Array.from(this.configuration.method).map(m => m.toUpperCase())).join(', ')}\
 requests are set to be executed. \
 Not performing HTTP ${transaction.request.method.toUpperCase()} request.\
 `);
       transaction.test = test;
       this.skipTransaction(transaction);
       return callback();
-    } if ((this.configuration.options.only.length > 0) && !(Array.from(this.configuration.options.only).includes(transaction.name))) {
+    } if ((this.configuration.only.length > 0) && !(Array.from(this.configuration.only).includes(transaction.name))) {
       logger.debug(`\
-Only '${this.configuration.options.only}' transaction is set to be executed. \
+Only '${this.configuration.only}' transaction is set to be executed. \
 Not performing HTTP request for '${transaction.name}'.\
 `);
       transaction.test = test;

lib/addHooks.js

@@ -32,7 +32,6 @@ function loadHookFile(hookfile, hooks) {
 }
 
 
-// Note: runner.configuration.options must be defined
 module.exports = function addHooks(runner, transactions, callback) {
   if (!runner.logs) { runner.logs = []; }
   runner.hooks = new Hooks({ logs: runner.logs, logger: reporterOutputLogger });
@@ -46,24 +45,24 @@ module.exports = function addHooks(runner, transactions, callback) {
   // Loading hookfiles from fs
   let hookfiles;
   try {
-    hookfiles = resolvePaths(runner.configuration.custom.cwd, runner.configuration.options.hookfiles);
+    hookfiles = resolvePaths(runner.configuration.custom.cwd, runner.configuration.hookfiles);
   } catch (err) {
     return callback(err);
   }
   logger.debug('Found Hookfiles:', hookfiles);
 
   // Override hookfiles option in configuration object with
   // sorted and resolved files
-  runner.configuration.options.hookfiles = hookfiles;
+  runner.configuration.hookfiles = hookfiles;
 
   // Clone the configuration object to hooks.configuration to make it
   // accessible in the node.js hooks API
   runner.hooks.configuration = clone(runner.configuration);
 
   // If the language is empty or it is nodejs
   if (
-    !runner.configuration.options.language
-    || runner.configuration.options.language === 'nodejs'
+    !runner.configuration.language
+    || runner.configuration.language === 'nodejs'
   ) {
     hookfiles.forEach(hookfile => loadHookFile(hookfile, runner.hooks));
     return callback();