Separate the task runner and the command line parser (#25)

* remove conditionl require in runner

add tasks module to load all tasks, throw if task does not exist

* change task signatures and action handlers

- tasks get plugin, run, and an options object
- action handler controls what goes into each task
- taskRunner wrapper moves the command object to the first argument

* change test command signature

allow files to be passed in, and pass all options to test:server and test:browser

* simplify the task runner

* fix typo in unknownOptions

* expose the task runner as the module's main

this way tasks can be run programatically without going through a cli parser

* add tests for task runner

* remove file passing for testAll

* add serverTestPaths to the plugin config

useful for overriding the value via a config file

* [config] plugin.serverTestPaths -> plugin.serverTestPatterns

bin/plugin-helpers.js

@@ -7,6 +7,14 @@ var run = require('../lib/run');
 var docs = require('../lib/docs');
 var enableCollectingUnknownOptions = require('../lib/enable_collecting_unknown_options');
 
+function taskRunner(fn) {
+  return function actionWrapper() {
+    var args = [].slice.apply(arguments);
+    var command = args.pop();
+    fn.apply(null, [command].concat(args));
+  };
+}
+
 program
   .version(pkg.version);
 
@@ -15,34 +23,51 @@ enableCollectingUnknownOptions(
     .command('start')
     .description('Start kibana and have it include this plugin')
     .on('--help', docs('start'))
-    .action(run('start'))
+    .action(taskRunner(function (command) {
+      run('start', {
+        flags: command.unknownOptions
+      });
+    }))
 );
 
 program
   .command('build')
   .description('Build a distributable archive')
   .on('--help', docs('build'))
-  .action(run('build'));
+  .action(taskRunner(function () {
+    run('build');
+  }));
 
 program
   .command('test')
   .description('Run the server and browser tests')
   .on('--help', docs('test/all'))
-  .action(run('test/all'));
+  .action(taskRunner(function (command, files) {
+    run('testAll');
+  }));
 
 program
   .command('test:browser')
   .description('Run the browser tests in a real web browser')
   .option('--dev', 'Enable dev mode, keeps the test server running')
   .option('-p, --plugins <plugin-ids>', 'Manually specify which plugins\' test bundles to run')
   .on('--help', docs('test/browser'))
-  .action(run('test/browser'));
+  .action(taskRunner(function (command) {
+    run('testBrowser', {
+      dev: Boolean(command.options.dev),
+      plugins: command.plugins,
+    });
+  }));
 
 program
   .command('test:server [files...]')
   .description('Run the server tests using mocha')
   .on('--help', docs('test/server'))
-  .action(run('test/server'));
+  .action(taskRunner(function (command, files) {
+    run('testServer', {
+      files: files
+    });
+  }));
 
 program
   .parse(process.argv);

lib/enable_collecting_unknown_options.js

@@ -4,7 +4,7 @@ module.exports = function enableCollectingUnknownOptions(command) {
   command.allowUnknownOption();
   command.parseOptions = function (argv) {
     let opts = origParse.call(this, argv);
-    this.unkownOptions = opts.unknown;
+    this.unknownOptions = opts.unknown;
     return opts;
   };
 };

lib/plugin_config.js

@@ -23,6 +23,7 @@ module.exports = function (root) {
   return Object.assign({
     root: root,
     kibanaRoot: resolve(root, '../kibana'),
+    serverTestPatterns: ['server/**/__tests__/**/*.js'],
     id: pkg.name,
     pkg: pkg,
     version: pkg.version,

lib/run.js

@@ -1,13 +1,10 @@
 var pluginConfig = require('./plugin_config');
+var tasks = require('./tasks');
 
-module.exports = function run(name) {
-  var action = require('../tasks/' + name);
-  return function () {
-    // call the action function with the plugin, then all
-    // renaining arguments from commander
-    var plugin = pluginConfig();
-    var args = [].slice.apply(arguments);
+module.exports = function run(name, options) {
+  var action = tasks[name];
+  if (!action) throw new Error('Invalid task: "' + name + '"');
 
-    action.apply(null, [plugin, run].concat(args));
-  };
+  var plugin = pluginConfig();
+  action(plugin, run, options);
 };

lib/run.spec.js

@@ -0,0 +1,30 @@
+/*eslint-env jest*/
+
+const testTask = jest.fn();
+const plugin = { id: 'testPlugin' };
+
+jest.mock('./plugin_config', () => () => plugin);
+jest.mock('./tasks', () => {
+  return { testTask };
+});
+const run = require('./run');
+
+describe('task runner', () => {
+  beforeEach(() => jest.resetAllMocks());
+
+  it('throw given an invalid task', function () {
+    const invalidTaskName = 'thisisnotavalidtasknameandneverwillbe';
+    const runner = () => run(invalidTaskName);
+
+    expect(runner).toThrow(/invalid task/i);
+  });
+
+  it('runs specified task with plugin and runner', function () {
+    run('testTask');
+
+    const args = testTask.mock.calls[0];
+    expect(testTask.mock.calls).toHaveLength(1);
+    expect(args[0]).toBe(plugin);
+    expect(args[1]).toBe(run);
+  });
+});

lib/tasks.js

@@ -0,0 +1,13 @@
+var buildTask = require('../tasks/build');
+var startTask = require('../tasks/start');
+var testAllTask = require('../tasks/test/all');
+var testBrowserTask = require('../tasks/test/browser');
+var testServerTask = require('../tasks/test/server');
+
+module.exports = {
+  build: buildTask,
+  start: startTask,
+  testAll: testAllTask,
+  testBrowser: testBrowserTask,
+  testServer: testServerTask,
+};

package.json

@@ -2,7 +2,7 @@
   "name": "@elastic/plugin-helpers",
   "version": "5.0.1-patch2",
   "description": "Just some helpers for kibana plugin devs.",
-  "main": "bin/plugin-helpers.js",
+  "main": "lib/run.js",
   "bin": {
     "plugin-helpers": "bin/plugin-helpers.js"
   },

tasks/build/build_action.spec.js

@@ -1,3 +1,4 @@
+/*eslint-env jest*/
 const resolve = require('path').resolve;
 const fs = require('fs');
 

tasks/start/start_action.js

@@ -1,13 +1,13 @@
 var execFileSync = require('child_process').execFileSync;
 
-module.exports = function (plugin, run, command) {
-  command = command || {};
+module.exports = function (plugin, run, options) {
+  options = options || {};
 
   var cmd = (process.platform === 'win32') ? 'bin\\kibana.bat' : 'bin/kibana';
   var args = ['--dev', '--plugin-path', plugin.root];
 
-  if (command.unkownOptions) {
-    args = args.concat(command.unkownOptions);
+  if (options.flags) {
+    args = args.concat(options.flags);
   }
 
   execFileSync(cmd, args, {

tasks/test/all/test_all_action.js

@@ -1,4 +1,4 @@
 module.exports = function testAllAction(plugin, run) {
-  run('test/server')();
-  run('test/browser')();
+  run('testServer');
+  run('testBrowser');
 };

tasks/test/browser/test_browser_action.js

@@ -1,20 +1,20 @@
 var execFileSync = require('child_process').execFileSync;
 
-module.exports = function testBrowserAction(plugin, run, command) {
-  command = command || {};
+module.exports = function testBrowserAction(plugin, run, options) {
+  options = options || {};
 
   var kbnServerArgs = [
     '--kbnServer.plugin-path=' + plugin.root
   ];
 
-  if (command.plugins) {
-    kbnServerArgs.push('--kbnServer.testsBundle.pluginId=' + command.plugins);
+  if (options.plugins) {
+    kbnServerArgs.push('--kbnServer.testsBundle.pluginId=' + options.plugins);
   } else {
     kbnServerArgs.push('--kbnServer.testsBundle.pluginId=' + plugin.id);
   }
 
   var cmd = 'npm';
-  var task = (command.dev) ? 'test:dev' : 'test:browser';
+  var task = (options.dev) ? 'test:dev' : 'test:browser';
   var args = ['run', task, '--'].concat(kbnServerArgs);
   execFileSync(cmd, args, {
     cwd: plugin.kibanaRoot,

tasks/test/server/test_server_action.js

@@ -2,12 +2,13 @@ var resolve = require('path').resolve;
 var delimiter = require('path').delimiter;
 var execFileSync = require('child_process').execFileSync;
 
-module.exports = function (plugin, run, files) {
+module.exports = function (plugin, run, options) {
   var kibanaBins = resolve(plugin.kibanaRoot, 'node_modules/.bin');
   var mochaSetupJs = resolve(plugin.kibanaRoot, 'test/mocha_setup.js');
+  options = options || {};
 
   var cmd = 'mocha';
-  var testPaths = (files.length) ? files : 'server/**/__tests__/**/*.js';
+  var testPaths = (options.files && options.files.length) ? options.files : plugin.serverTestPatterns;
   var args = ['--require', mochaSetupJs].concat(testPaths);
   var path = `${kibanaBins}${delimiter}${process.env.PATH}`;